Further, we expect the launcher to use the supported JNI interface
(as
other processes would), not the internal JVM interface that exists
for
the JDK sources to communicate with the JVM.
I think that we do not use JVM interface if we add new method in
LauncherHelper as below:
----------------
diff -r f02139a1ac84
src/java.base/share/classes/sun/launcher/LauncherHelper.java
--- a/src/java.base/share/classes/sun/launcher/LauncherHelper.java
Wed Apr 13 14:19:30 2016 +0000
+++ b/src/java.base/share/classes/sun/launcher/LauncherHelper.java
Sat Apr 16 11:25:53 2016 +0900
@@ -960,4 +960,8 @@
else
return md.toNameAndVersion() + " (" + loc + ")";
}
+
+ static void setNativeThreadName(String name) {
+ Thread.currentThread().setName(name);
+ }
}
diff -r f02139a1ac84 src/java.base/share/native/libjli/java.c
--- a/src/java.base/share/native/libjli/java.c Wed Apr 13 14:19:30
2016 +0000
+++ b/src/java.base/share/native/libjli/java.c Sat Apr 16 11:25:53
2016 +0900
@@ -125,6 +125,7 @@
static void PrintUsage(JNIEnv* env, jboolean doXUsage);
static void ShowSettings(JNIEnv* env, char *optString);
static void ListModules(JNIEnv* env, char *optString);
+static void SetNativeThreadName(JNIEnv* env, char *name);
static void SetPaths(int argc, char **argv);
@@ -325,6 +326,7 @@
* mainThread.isAlive() to work as expected.
*/
#define LEAVE() \
+ SetNativeThreadName(env, "DestroyJavaVM"); \
do { \
if ((*vm)->DetachCurrentThread(vm) != JNI_OK) { \
JLI_ReportErrorMessage(JVM_ERROR2); \
@@ -488,6 +490,9 @@
mainArgs = CreateApplicationArgs(env, argv, argc);
CHECK_EXCEPTION_NULL_LEAVE(mainArgs);
+ /* Set native thread name. */
+ SetNativeThreadName(env, "main");
+
/* Invoke main method. */
(*env)->CallStaticVoidMethod(env, mainClass, mainID, mainArgs);
@@ -1686,6 +1691,22 @@
joptString);
}
+/**
+ * Set native thread name as possible.
+ */
+static void
+SetNativeThreadName(JNIEnv *env, char *name)
+{
+ jmethodID setNativeThreadNameID;
+ jstring nameString;
+ jclass cls = GetLauncherHelperClass(env);
+ NULL_CHECK(cls);
+ NULL_CHECK(setNativeThreadNameID =
(*env)->GetStaticMethodID(env, cls,
+ "setNativeThreadName", "(Ljava/lang/String;)V"));
+ NULL_CHECK(nameString = (*env)->NewStringUTF(env, name));
+ (*env)->CallStaticVoidMethod(env, cls, setNativeThreadNameID,
nameString);
+}
+
/*
* Prints default usage or the Xusage message, see
sun.launcher.LauncherHelper.java
*/
----------------
So I want to add new arg to JVM_SetNativeThreadName().
However this is still a change to the exported JVM interface and so
has to be approved.
Do you mean that this change needs CCC?
Thanks,
Yasumasa
[1]
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-April/019034.html
On 2016/04/16 7:26, David Holmes wrote:
On 15/04/2016 11:20 PM, Yasumasa Suenaga wrote:
Hi David,
I think it is a bug based on the comment here:
JavaThread(bool is_attaching_via_jni = false); // for main
thread and
JNI attached threads
I filed it to JBS as JDK-8154331.
I will send review request to hotspot-runtime-dev.
Though that will introduce a change in behaviour by itself as
setName
will no longer set the native name for the main thread.
I know.
That change in behaviour may be a problem, it could be considered a
regression that setName stops setting the native thread main, even
though we never really intended it to work in the first place. :(
Such
a change needs to go through CCC.
I checked changeset history.
JVM_SetNativeThreadName() was introduced in JDK-7098194, and it is
backported JDK 8.
Yes this all came in as part of the OSX port in 7u2.
However, this function seems to be called from
Thread#setNativeName()
only.
In addition, Thread#setNativeName() is private method.
Thus I think that we can add an argument to
JVM_SetNativeThreadName()
for force setting.
(e.g. "bool forced")
It makes a change of JVM API.
However, this function is NOT public, so I think we can add one
more
argument.
What do you think about this?
If it is accepted, we can set native thread name from Java
launcher.
The private/public aspect of the Java API is not really at issue.
Yes
we would add another arg to the JVM function to allow it to apply to
JNI-attached threads as well (I'd prefer the arg reflect that not
just
"force"). However this is still a change to the exported JVM
interface
and so has to be approved. Further, we expect the launcher to use
the
supported JNI interface (as other processes would), not the internal
JVM interface that exists for the JDK sources to communicate with
the
JVM.
David
-----
Thanks,
Yasumasa
On 2016/04/15 19:16, David Holmes wrote:
Hi Yasumasa,
On 15/04/2016 6:53 PM, Yasumasa Suenaga wrote:
Hi David,
The fact that the "main" thread is not tagged as being a
JNI-attached
thread seems accidental to me
Should I file it to JBS?
I think it is a bug based on the comment here:
JavaThread(bool is_attaching_via_jni = false); // for main
thread and
JNI attached threads
Though that will introduce a change in behaviour by itself as
setName
will no longer set the native name for the main thread.
I think that we can fix as below:
---------------
diff -r 52aa0ee93b32 src/share/vm/runtime/thread.cpp
--- a/src/share/vm/runtime/thread.cpp Thu Apr 14 13:31:11 2016
+0200
+++ b/src/share/vm/runtime/thread.cpp Fri Apr 15 17:50:10 2016
+0900
@@ -3592,7 +3592,7 @@
#endif // INCLUDE_JVMCI
// Attach the main thread to this os thread
- JavaThread* main_thread = new JavaThread();
+ JavaThread* main_thread = new JavaThread(true);
main_thread->set_thread_state(_thread_in_vm);
main_thread->initialize_thread_current();
// must do this before set_active_handles
@@ -3776,6 +3776,9 @@
// Notify JVMTI agents that VM initialization is complete -
nop if
no agents.
JvmtiExport::post_vm_initialized();
+ // Change attach status to "attached"
+ main_thread->set_done_attaching_via_jni();
+
I think we can do this straight after the JavaThread constructor.
if (TRACE_START() != JNI_OK) {
vm_exit_during_initialization("Failed to start tracing
backend.");
}
---------------
If it wants to name its native threads then it is free to do so,
Currently, JVM_SetNativeThreadName() cannot change native thread
name
when the caller thread is JNI-attached thread.
However, I think that it should be changed if Java developer
calls
Thread#setName() explicitly.
It is not the same of changing native thread name at
Threads::create_vm().
If it is allowed, I want to fix SetNativeThreadName() as below.
What do you think about this?
The decision to not change the name of JNI-attached threads was a
deliberate one** - this functionality originated with the OSX port
and
it was reported that the initial feedback with this feature was to
ensure it didn't mess with thread names that had been set by the
host
process. If we do as you propose then we will just have an
inconsistency for people to complain about: "why does my native
thread
only have a name if I call cur.setName(cur.getName()) ?"
** If you follow the bugs and related discussions on this, the
semantics and limitations (truncation, current thread only,
non-JNI
threads only) of setting the native thread name were supposed
to be
documented in the release notes - but as far as I can see that
never
happened. :(
My position on this remains that if it is desirable for the main
thread (and DestroyJavaVM thread) to have native names then the
launcher needs to be setting them using the available platform
APIs.
Unfortunately this is complicated - as evidenced by the VM code
for
this - due to the need to verify API availability.
Any change in behaviour in relation to Thread.setName would have
to go
through our CCC process I think. But a change in the launcher
would
not.
Sorry.
David
-----
---------------
--- a/src/share/vm/prims/jvm.cpp Thu Apr 14 13:31:11 2016
+0200
+++ b/src/share/vm/prims/jvm.cpp Fri Apr 15 17:50:10 2016
+0900
@@ -3187,7 +3187,7 @@
JavaThread* thr = java_lang_Thread::thread(java_thread);
// Thread naming only supported for the current thread,
doesn't
work
for
// target threads.
- if (Thread::current() == thr &&
!thr->has_attached_via_jni()) {
+ if (Thread::current() == thr) {
// we don't set the name of an attached thread to avoid
stepping
// on other programs
const char *thread_name =
java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(name));
---------------
Thanks,
Yasumasa
On 2016/04/15 13:32, David Holmes wrote:
On 15/04/2016 1:11 AM, Yasumasa Suenaga wrote:
Roger,
Thanks for your comment!
David,
I'll wait to see what Kumar thinks about this. I don't like
exposing
a new JVM function this way.
I tried to call Thread#setName() after initializing VM (before
calling
main method),
I could set native thread name.
However, DestroyJavaVM() calls AttachCurrentThread(). So we
can't
set
native thread name for DestroyJavaVM.
Right - I came to the same realization earlier this morning.
Which,
unfortunately, takes me back to the basic premise here that we
don't
set the name of threads not created by the JVM. The fact that
the
"main" thread is not tagged as being a JNI-attached thread seems
accidental to me - so JVM_SetNativeThreadName is only working by
accident for the initial attach, and can't be used for the
DestroyJavaVM part - which leaves the thread inconsistently
named at
the native level.
I'm afraid my view here is that the launcher has to be treated
like
any other process that might host a JVM. If it wants to name its
native threads then it is free to do so, but I would not be
exporting
a function from the JVM to do that - it would have to use the OS
specific API's for that on a platform-by-platform basis.
Sorry.
David
-----
Thanks,
Yasumasa
On 2016/04/14 23:24, Roger Riggs wrote:
Hi,
Comments:
jvm.h: The function names are too similar but perform
different
functions:
-JVM_SetNativeThreadName0 vs JVM_SetNativeThreadName
- The first function applies to the current thread, the
second
one a
specific java thread.
It would seem useful for there to be a comment somewhere
about
what
the new function does.
windows/native/libjli/java_md.c: line 408 casts to (void*)
instead of
(SetNativeThreadName0_t)
as is done on unix and mac.
- macosx/native/libjli/java_md_macosx.c:
- 737: looks wrong to overwriteifn->GetCreatedJavaVMs
used at
line 730
- 738 Incorrect indentation; if possible keep the cast
on the
same
line as dlsym...
$.02, Roger
On 4/14/2016 9:32 AM, Yasumasa Suenaga wrote:
That is an interesting question which I haven't had time to
check -
sorry. If the main thread is considered a JNI-attached
thread
then
my suggestion wont work. If it isn't then my suggestion
should
work
(but it means we have an inconsistency in our treatment of
JNI-attached threads :( )
I ran following program on JDK 9 EA b112, and I confirmed
native
thread name (test) was set.
---------
public class Sleep{
public static void main(String[] args) throws Exception{
Thread.currentThread().setName("test");
Thread.sleep(3600000);
}
}
---------
I'll wait to see what Kumar thinks about this. I don't like
exposing
a new JVM function this way.
I will update webrev after hearing Kumar's comment.
Thanks,
Yasumasa
On 2016/04/14 21:32, David Holmes wrote:
On 14/04/2016 1:52 PM, Yasumasa Suenaga wrote:
Hi,
On 2016/04/14 9:34, David Holmes wrote:
Hi,
On 14/04/2016 1:28 AM, Yasumasa Suenaga wrote:
Hi David,
Thanks for your comment.
I exported new JVM function to set native thread name,
and JLI
uses it
in new webrev.
First the launcher belongs to another team so core-libs
will
need to
review and approve this (in particular Kumar) - now cc'd.
Thanks!
I'm waiting to review :-)
Personally I would have used a Java upcall to
Thread.setName
rather
than exporting JVM_SetNativeThreadName. No hotspot changes
needed in
that case.
As I wrote [1] in JBS, I changed to use Thread#setName() in
Thread#init(),
but I could not change native thread name.
At Thread.init time the thread is not alive, which is why
the
native
name is not set.
I guess that caller of main() is JNI attached thread.
That is an interesting question which I haven't had time to
check -
sorry. If the main thread is considered a JNI-attached
thread
then
my suggestion wont work. If it isn't then my suggestion
should
work
(but it means we have an inconsistency in our treatment of
JNI-attached threads :( )
I'll wait to see what Kumar thinks about this. I don't like
exposing
a new JVM function this way.
Thanks,
David
-----
Thus I think that we have to provide a function to set
native
thread name.
Thanks,
Yasumasa
[1]
https://bugs.openjdk.java.net/browse/JDK-8152690?focusedCommentId=13926851&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13926851
Thanks,
David
Could you review again?
- hotspot:
http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.02/hotspot/
- jdk:
http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.02/jdk/
Thanks,
Yasumasa
On 2016/04/13 22:00, David Holmes wrote:
I'll answer on this original thread as well ...
Hi Yasumasa,
Please see my updates to the bug (sorry have been on
vacation).
This
needs to be done in the launcher to be correct as we
do not
set the
name of threads that attach via JNI, which includes the
"main"
thread.
David
On 31/03/2016 9:49 AM, Yasumasa Suenaga wrote:
Thanks Robbin,
I'm waiting a sponsor and more reviewer :-)
Yasumasa
2016/03/31 5:58 "Robbin Ehn" <robbin....@oracle.com>:
FYI: I'm not a Reviewer.
/Robbin
On 03/30/2016 10:55 PM, Robbin Ehn wrote:
Thanks, looks good.
/Robbin
On 03/30/2016 03:47 PM, Yasumasa Suenaga wrote:
Hi,
I uploaded new webrev.
Could you review it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.01/
Thanks,
Yasumasa
On 2016/03/30 19:10, Robbin Ehn wrote:
Hi,
On 03/30/2016 11:41 AM, Yasumasa Suenaga wrote:
Hi Robbin,
2016/03/30 18:22 "Robbin Ehn"
<robbin....@oracle.com
<mailto:robbin....@oracle.com>>:
>
> Hi Yasumasa,
>
>
> On 03/25/2016 12:48 AM, Yasumasa Suenaga
wrote:
>>
>> Hi Robbin,
>> 2016/03/25 1:51 "Robbin Ehn"
<robbin....@oracle.com
<mailto:robbin....@oracle.com>
>> <mailto:robbin....@oracle.com
<mailto:robbin....@oracle.com>>>:
>>
>> >
>> > Hi Yasumasa,
>> >
>> > I'm not sure why you don't set it:
>> >
>> > diff -r ded6ef79c770
src/share/vm/runtime/thread.cpp
>> > --- a/src/share/vm/runtime/thread.cpp
Thu
Mar 24
13:09:16 2016
+0000
>> > +++ b/src/share/vm/runtime/thread.cpp
Thu
Mar 24
17:40:09 2016
+0100
>> > @@ -3584,6 +3584,7 @@
>> > JavaThread* main_thread = new
JavaThread();
>> >
main_thread->set_thread_state(_thread_in_vm);
>> > main_thread->initialize_thread_current();
>> > +
main_thread->set_native_thread_name("main");
>> > // must do this before
set_active_handles
>> > main_thread->record_stack_base_and_size();
>> >
main_thread->set_active_handles(JNIHandleBlock::allocate_block());
>> >
>> > here instead? Am I missing something?
>>
>> Native thread name is the same to thread
name in
Thread
class.
>> It is set in c'tor in Thread or setName().
>> If you create new thread in Java app, native
thread
name
will be
set at
>> startup. However, main thread is already
starte
in VM.
>> Thread name for "main" is set in
create_initial_thread().
>> I think that the place of setting thrrad name
should
be the
same.
>
>
> Yes, I see your point. But then something like
this is
nicer, no?
>
> --- a/src/share/vm/runtime/thread.cpp Tue
Mar 29
09:43:05
2016
+0200
> +++ b/src/share/vm/runtime/thread.cpp Wed
Mar 30
10:51:12
2016
+0200
> @@ -981,6 +981,7 @@
> // Creates the initial Thread
> static oop create_initial_thread(Handle
thread_group,
JavaThread*
thread,
> TRAPS) {
> + static const char* initial_thread_name =
"main";
> Klass* k =
SystemDictionary::resolve_or_fail(vmSymbols::java_lang_Thread(),
true,
CHECK_NULL);
> instanceKlassHandle klass (THREAD, k);
> instanceHandle thread_oop =
klass->allocate_instance_handle(CHECK_NULL);
> @@ -988,8 +989,10 @@
> java_lang_Thread::set_thread(thread_oop(),
thread);
> java_lang_Thread::set_priority(thread_oop(),
NormPriority);
> thread->set_threadObj(thread_oop());
> -
> - Handle string =
java_lang_String::create_from_str("main",
CHECK_NULL);
> +
> +
thread->set_native_thread_name(initial_thread_name);
> +
> + Handle string =
java_lang_String::create_from_str(initial_thread_name,
CHECK_NULL);
>
> JavaValue result(T_VOID);
> JavaCalls::call_special(&result, thread_oop,
Okay, I will upload new webrev later.
Thanks!
>> > The launcher seem to name itself 'java'
and
naming
this
thread
just
>> > 'main' is confusing to me.
>> >
>> > E.g. so main thread of the process (and
thus
the
process) is
'java' but
>> > first JavaThread is 'main'.
>>
>> The native main thread in the process is not
JavaThread.
It is
waiting
>> for ending of Java main thread with
pthread_join().
>> set_native_thread_name() is for JavaThread.
So I
think that
we do
not
>> need to call it for native main thread.
>
>
> Not sure if we can change it anyhow, since we
want
java and
native
name to be the same and java thread name might
have
some
dependents.
>
> The name is visible in e.g. /proc.
>
> $ ps H -C java -o 'pid tid comm' | head -4
> PID TID COMMAND
> 6423 6423 java
> 6423 6424 main
> 6423 6425 GC Thread#0
>
> It would be nice with something like 'Java
Main
Thread'.
I do not think so.
Native main thread might not be a Java launcher -
e.g.
Apache
commons-daemon, JNI application, etc.
If you want to change native main thread name, I
think
that we
have to
change Java launcher code.
Should I include it in new webrev?
No
Thanks again!
/Robbin
Thanks,
Yasumasa
> Thanks
>
> /Robbin
>
>
>>
>> Thanks,
>>
>> Yasumasa
>>
>> > Thanks!
>> >
>> > /Robbin
>> >
>> > On 03/24/2016 03:26 PM, Yasumasa Suenaga
wrote:
>> > > Hi all,
>> > >
>> > > HotSpot for Linux will set thread name
via
pthread_setname_np().
>> > > However, main thread does not have it.
>> > >
>> > > All JavaThread have native name, and
main
thread is
JavaThread.
>> > > For consistency, main thread should have
native
thread
name.
>> > >
>> > > I uploaded a webrev. Could you review
it?
>> > >
>> > >
http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.00/
>> > >
>> > > I cannot access JPRT.
>> > > So I need a sponsor.
>> > >
>> > >
>> > > Thanks,
>> > >
>> > > Yasumasa
>> > >
>>
