Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-7102541

RFE: os::set_native_thread_name() cleanups

XMLWordPrintable

    • b37
    • generic
    • generic
    • Not verified

        The MacOS X Port project used the following changeset for the initial
        push from macosx-port/hotspot -> HSX-23:

            http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/436b4a3231bf

            7098194: integrate macosx-port changes

        and that changeset includes a new method for setting a native thread
        name.


        David H made the following code review comment:

        On 10/11/11 10:48 PM, David Holmes wrote:
        >
        > src/os/<os>/vm/os_<os>.cpp
        >
        > + void os::set_native_thread_name(const char *name) {
        > + // Not yet implemented.
        > + return;
        > + }
        >
        > I hate seeing "shared" code that is not implemented on 3 out of
        > 4 supported platforms. Though it seems Linux will also support
        > pthread_setname_np as of March 2010 (not sure which kernel or
        > glibc versions needed). If not for the fact this also adds an
        > exported JVM method to set the native thread name, we could
        > otherwise bury this in the platform specific native thread
        > creation code (which might also obviate the need for the new
        > _is_attached logic - see next point). I also wonder what Java
        > code is using this new JVM API?
        >
        > Also wondering where the "current thread only" restriction came about?


        My replies (guesses) to David H:

        > I suspect that this new API is used by other pieces of the MacOS X
        > port project, but I don't know that for sure. Again, we need an
        > Apple person to jump in here.
        >
        > If the API is restricted to the "current thread", then we don't have
        > to worry about races and locking. Also, I'm wondering why there is no
        > return code.


        Here is a reply from Roger H:

        On 10/12/11 12:34 PM, roger hoover wrote:
        > The native thread name addition was a huge win, both for us
        > debugging and for our developer customers. The only complaint
        > we had was from other projects whose code attached to Java and
        > (in initial revisions) had their threads renamed. It is exported
        > so that Thread.java can set the native thread name.
        >
        > I have no explanation for the lack of a return code. I can only
        > surmise that it is an oversight that was not caught by the compiler.
        >
        > The other pieces are:
        > macosx-port/jdk/src/share/native/java/lang/Thread.c:
        > {"getThreads", "()[" THD, (void *)&JVM_GetAllThreads},
        > {"dumpThreads", "([" THD ")[[" STE, (void *)&JVM_DumpThreads},
        > --> {"setNativeName", "(" STR ")V", (void *)&JVM_SetNativeThreadName},
        > };
        > -and-
        > macosx-port/jdk/src/share/classes/java/lang/Thread.java
        > /**
        > * Changes the name of this thread to be equal to the argument
        > * <code>name</code>.
        > * <p>
        > * First the <code>checkAccess</code> method of this thread is called
        > * with no arguments. This may result in throwing a
        > * <code>SecurityException</code>.
        > *
        > * @param name the new name for this thread.
        > * @exception SecurityException if the current thread cannot modify this
        > * thread.
        > * @see #getName
        > * @see #checkAccess()
        > */
        > public final void setName(String name) {
        > checkAccess();
        > this.name = name.toCharArray();
        > --> if (threadStatus != 0) {
        > --> setNativeName(name);
        > --> }
        > }


        Here is a reply from Paul H on the thread:

        On 10/12/11 2:56 PM, Paul Hohensee wrote:
        > If set_native_thread_name() is an implementation of an extension to Thread,
        > then I'd recommend leaving it and the supporting code out (and there's a lot of
        > it, vis jvm.cpp as you point out) until the core libs people figure out how
        > they want to handle it. Maybe file an RFE for the work.


        My reply to Paul H:

        On 10/12/11 4:33 PM, Daniel D. Daugherty wrote:
        > If I remove that support, then I won't be able to use the resulting
        > VM with the rest of the macosx-port forest, right? Part of my
        > bring up strategy relies on my being able to use the HSX-23 port
        > bits with the rest of the macosx-port built bits...


        Another comment from Paul H:

        > True. But it may not be used in it's current form.


        And my reply about it being used:

        On 10/12/11 4:53 PM, Daniel D. Daugherty wrote:
        > I think it is:
        >
        > jdk/src/share/javavm/export/jvm.h:JVM_SetNativeThreadName(JNIEnv *env, jobject jthread, jstring name);
        > jdk/src/share/native/java/lang/Thread.c: {"setNativeName", "(" STR ")V", (void *)&JVM_SetNativeThreadName},
        > jdk/src/share/classes/java/lang/Thread.java: setNativeName(name);
        > jdk/src/share/classes/java/lang/Thread.java: private native void setNativeName(String name);


        Tom R's addition to the thread:

        On 10/12/11 5:39 PM, Tom Rodriguez wrote:
        > The important point is that it doesn't expose any new API.
        > It's just a side effect of calling the regular setName.
        >
        > I was a bit dubious about its utility so I googled
        > pthread_setname_np and found out it allows debuggers to report
        > useful names for threads instead of just the thread number.
        > On linux it's even accessible through /proc. Presumably the
        > mac does something similar. It seems like a fairly cool
        > feature. We might want to do it on linux as well and maybe
        > even put proper names on our internal threads (eventually).


        And Roger H's reply to Tom:

        On 10/12/11 5:49 PM, roger hoover wrote:
        > It makes a big difference. gdb tells you what the threads are,
        > and we get that info as well in native crash dumps. Here is a
        > snippet from a crash dump on our jdk6 implementation. This also
        > helps JNI developers because they can understand in which thread
        > their code is crashing:
        >
        > Thread 14: Java: VM Thread
        > 0 libSystem.B.dylib 0x00007fff802abd7a mach_msg_trap + 10
        > 1 libSystem.B.dylib 0x00007fff802ac3ed mach_msg + 59
        > 2 libclient64.dylib 0x000000010100d8f5 jio_snprintf + 37807
        > 3 libclient64.dylib 0x000000010102c4aa jio_vsnprintf + 26334
        > 4 libclient64.dylib 0x000000010100d25d jio_snprintf + 36119
        > 5 libclient64.dylib 0x000000010100d103 jio_snprintf + 35773
        > 6 libclient64.dylib 0x00000001010a3ef7 JVM_Lseek + 188645
        > 7 libclient64.dylib 0x00000001010a3c47 JVM_Lseek + 187957
        > 8 libclient64.dylib 0x000000010100d1c4 jio_snprintf + 35966
        > 9 libSystem.B.dylib 0x00007fff802e4fd6 _pthread_start + 331
        > 10 libSystem.B.dylib 0x00007fff802e4e89 thread_start + 13
        >
        > Thread 15: Java: Reference Handler
        > 0 libSystem.B.dylib 0x00007fff802abd7a mach_msg_trap + 10
        > 1 libSystem.B.dylib 0x00007fff802ac3ed mach_msg + 59
        > 2 libclient64.dylib 0x000000010100d863 jio_snprintf + 37661
        > 3 libclient64.dylib 0x000000010100d723 jio_snprintf + 37341
        > 4 libclient64.dylib 0x00000001010b24ff JVM_MonitorWait + 3999
        > 5 libclient64.dylib 0x00000001010b198c JVM_MonitorWait + 1068
        > 6 libclient64.dylib 0x00000001010b15fa JVM_MonitorWait + 154
        > 7 libjvmlinkage.dylib 0x0000000100093b9b JVM_MonitorWait + 59
        > 8 ??? 0x0000000104011d6e 0 + 4362149230
        > 9 ??? 0x000000010400685a 0 + 4362102874
        > 10 ??? 0x000000010400685a 0 + 4362102874
        > 11 ??? 0x0000000104001438 0 + 4362081336
        > 12 libclient64.dylib 0x00000001010a50ba JVM_Lseek + 193192
        > 13 libclient64.dylib 0x00000001010b1148 JVM_StartThread + 2565
        > 14 libclient64.dylib 0x00000001010b103e JVM_StartThread + 2299
        > 15 libclient64.dylib 0x00000001010b0fde JVM_StartThread + 2203
        > 16 libclient64.dylib 0x00000001010b0e80 JVM_StartThread + 1853
        > 17 libclient64.dylib 0x00000001010b0c95 JVM_StartThread + 1362
        > 18 libclient64.dylib 0x000000010100d1c4 jio_snprintf + 35966
        > 19 libSystem.B.dylib 0x00007fff802e4fd6 _pthread_start + 331
        > 20 libSystem.B.dylib 0x00007fff802e4e89 thread_start + 13
        >
        > Thread 16: Java: Finalizer
        > 0 libSystem.B.dylib 0x00007fff802abd7a mach_msg_trap + 10
        > 1 libSystem.B.dylib 0x00007fff802ac3ed mach_msg + 59
        > 2 libclient64.dylib 0x000000010100d863 jio_snprintf + 37661
        > 3 libclient64.dylib 0x000000010100d723 jio_snprintf + 37341
        > 4 libclient64.dylib 0x00000001010b24ff JVM_MonitorWait + 3999
        > 5 libclient64.dylib 0x00000001010b198c JVM_MonitorWait + 1068
        > 6 libclient64.dylib 0x00000001010b15fa JVM_MonitorWait + 154
        > 7 libjvmlinkage.dylib 0x0000000100093b9b JVM_MonitorWait + 59
        > 8 ??? 0x0000000104011d6e 0 + 4362149230
        > 9 ??? 0x000000010400685a 0 + 4362102874
        > 10 ??? 0x00000001040069b3 0 + 4362103219
        > 11 ??? 0x00000001040069b3 0 + 4362103219
        > 12 ??? 0x0000000104001438 0 + 4362081336
        > 13 libclient64.dylib 0x00000001010a50ba JVM_Lseek + 193192
        > 14 libclient64.dylib 0x00000001010b1148 JVM_StartThread + 2565
        > 15 libclient64.dylib 0x00000001010b103e JVM_StartThread + 2299
        > 16 libclient64.dylib 0x00000001010b0fde JVM_StartThread + 2203
        > 17 libclient64.dylib 0x00000001010b0e80 JVM_StartThread + 1853
        > 18 libclient64.dylib 0x00000001010b0c95 JVM_StartThread + 1362
        > 19 libclient64.dylib 0x000000010100d1c4 jio_snprintf + 35966
        > 20 libSystem.B.dylib 0x00007fff802e4fd6 _pthread_start + 331
        > 21 libSystem.B.dylib 0x00007fff802e4e89 thread_start + 13
        >
        > Thread 17: Dispatch queue: com.apple.libdispatch-manager


        And another comment from David H:

        On 10/12/11 7:59 PM, David Holmes wrote:
        > - set_native_thread_name:
        >
        > Ok so I have to go back to earlier emails here. Makes sense that
        > Thread.setName is augmented to call JVM_SetNativeThreadName.
        > However I don't like the semantics of this particular addition.
        > setName has no usage restrictions on it, but set_native_thread_name
        > is a no-op unless called by the current thread. By restricting it
        > to the current thread it simplifies synchronization logic, but it
        > seems a somewhat arbitrary restriction in terms of the functionality,
        > given the more general nature of the setName API. I guess this is
        > Yet Another RFE: set_native_thread_name should not be restricted
        > to only the current thread.
        >
        > BTW: are the JDK changes associated with this also being pushed at
        > some stage? If so are the hotspot changes being pushed first?


        My replies to David H:

        On 10/13/11 8:07 AM, Daniel D. Daugherty wrote:
        >> I guess this is Yet Another RFE: set_native_thread_name should not
        >> be restricted to only the current thread.
        >
        > We definitely need to get some clarity about the addition of
        > JVM_SetNativeThreadName(). I'm not fond of APIs without return
        > values so that's one thing to log in the new bug. The "must be
        > current thread" semantic should also be hashed out/clarified.
        >
        > Like any API between the JDK and HotSpot, changing it will be a
        > pain and require coordination.
        >
        >> BTW: are the JDK changes associated with this also being pushed at some stage?
        >
        > Yes, but the exact process has not been finalized. Since there
        > are changes being made/coordinated by multiple teams, I'm not
        > sure which JDK8 sub-repos will be used.
        >
        >
        >> If so are the hotspot changes being pushed first?
        >
        > I'm planning to push this changeset to RT_Baseline today.

              gziemski Gerard Ziemski
              dcubed Daniel Daugherty
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

                Created:
                Updated:
                Resolved:
                Imported:
                Indexed: