I'm OK with it and, in fact, I recommend doing the incremental
improvement via 7007254 and leaving 5049313 for another day.
Tomas and I IM'ed about it yesterday.
Dan
On 3/31/2011 9:20 AM, Kelly O'Hair wrote:
It's ok with me, but I'm not in the Serviceability team anymore.
Make sure Dan is ok with it, or at least knows about it.
-kto
On Mar 31, 2011, at 1:58 AM, Tomas Hurka wrote:
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 comman
d 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:[email protected]>
NetBeans Profiler http://profiler.netbeans.org
VisualVM http://visualvm.java.net
Software Developer
Oracle, Praha Czech Republic
|