code review for CR7007254

[Tomas Hurka]

Hi,
I would like to resubmit this code review. I had the private discussion with Dan Daugherty and we agreed that it would be good to integrate the current fix I proposed below. Our reasoning is as follow:
1) It fixes the reported problem
2) The fix is not intended to address 5049313 (which talks about mUTF-8 encoding), but is instead an incremental improvement to the platform encoding support that already exists
3) the size of the change with correct mUTF-8 encoding will be much bigger and I am afraid that this can create incompatibility in Attach API. So it is probably not a good idea to change it for JDK 7 at this time

So Alan and Kelly, what do you think? Are you OK with this 'incremental improvement to the platform encoding support'?

Thanks.

On 4 Jan 2011, at 14:05, Tomas Hurka wrote:

> Hello,
> I would like to as for review of the fix for CR7007254 - A NullPointerException occurs with jvisualvm placed under a dir. including Japanese chars.
> 
> Webrev: http://cr.openjdk.java.net/~thurka/7007254/webrev.00/
> 
> Note: To reproduce the issue, path to jfluid-server-15.jar must contain Japanese characters.
> 
> Details:
> jfluid-server-15.jar is javaagent library loaded into target JVM via Attach API. It calls ClassLoader.getSystemClassLoader().getResource("org/netbeans/lib/profiler/server/ProfilerActivate15.class") as part of agentmain method. org/netbeans/lib/profiler/server/ProfilerActivate15.class is part of jfluid-server-15.jar, but ClassLoader.getSystemClassLoader().getResource("...") returns null because jfluid-server-15.jar is added incorrectly to the system classloader classpath. The problem is in JvmtiEnv::AddToSystemClassLoaderSearch(), where parameter 'segment' is encoded in platform dependent encoding (on Windows and Solaris), but it is treated as UTF-8 when converted to java.lang.String object via call to java_lang_String::create_from_str(segment, THREAD). I tested the the fix on Windows. It also works on Linux, even though the 'segment' is encoded in UTF-8, probably because on Linux native encoding is UTF-8 anyway. Maybe it would be useful to unify encoding of Attach API command and arguments and use platform dependent encoding on all platforms. Currently the encoding for Windows is done via JNU_GetStringPlatformChars in jdk/src/windows/native/sun/tools/attach/WindowsVirtualMachine.c in method Java_sun_tools_attach_WindowsVirtualMachine_enqueue for Solaris in jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c in method Java_sun_tools_attach_SolarisVirtualMachine_enqueue. The Linux part is done differently at sun.tools.attach.LinuxVirtualMachine.execute() and encodes Attach API command and arguments in UTF-8.

--
Tomas Hurka   <mailto:tomas.hurka at oracle.com>
NetBeans Profiler http://profiler.netbeans.org
VisualVM http://visualvm.java.net
Software Developer
Oracle, Praha Czech Republic