Re: PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
Hi Yasumasa,
Changes look and and passed all my testing.
thanks,
Chris
On 7/19/18 10:13 PM, Yasumasa Suenaga wrote:
Hi Chris,
Thank you for your comment.
I uploaded new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.02/
I tested my change on Linux x64, but I cannot check it on other
platform (includes older Linux).
However SA tests are included in HotSpot tier 1 tests. Tests on submit
repo work fine with this change
(mach5-one-ysuenaga-JDK-8205992-20180720-0305-31840).
Thanks,
Yasumasa
2018-07-20 3:26 GMT+09:00 Chris Plummer :
Hi Yasumasa,
84 // It maps the LWPID in the host to it in the container.
"it" -> "the PID"
286 // Get LWPID in the host from the container's LWPID.
287 public int getHostPID(int id) {
288 try {
289 return nspidMap.get(id);
290 } catch (NullPointerException e) {
291 return -1;
292 }
293 }
What is the source of the NPE here? Is it because nspidMap was never
initialized because the process is not in a container? In that case I think
you should be checking for null rather than having an NPE be part of normal
execution.
42 int hostPID =
((LinuxDebuggerLocal)debugger).getHostPID(pid);
43 if (hostPID != -1) {
44 pid = hostPID;
45 }
A comment here would be helpful.
The rest looks good. I should probably run it through some internal testing.
Let me know when you have a final webrev.
thanks,
Chris
On 7/18/18 5:59 AM, Yasumasa Suenaga wrote:
PING:
Could you review it?
JBS:https://bugs.openjdk.java.net/browse/JDK-8205992
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/
This change has been reviewed by Jini.
We need a Reviewer.
Thanks,
Yasumasa
On 2018/07/12 13:42, Yasumasa Suenaga wrote:
Thanks Jini,
I uploaded new webrev. It contains some comments and removing extra
space.
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/
Yasumasa
2018-07-12 2:32 GMT+09:00 Jini George :
Hi Yasumasa,
This looks good to me except for one nit. And some more comments would
help.
For e.g., it would help to say that NSPidMap is to map the host to
container
lwpids.
The nit:
*
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html
Line 253: extra space after the parentheses
Thanks,
Jini.
On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote:
PING: Could you review it?
JBS: https://bugs.openjdk.java.net/browse/JDK-8205992
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/
Thanks,
Yasumasa
On 2018/06/28 22:12, Yasumasa Suenaga wrote:
Hi all,
Please review this change.
JBS: https://bugs.openjdk.java.net/browse/JDK-8205992
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/
I tried to attach jhsdb to java process in docker container from
container host, but it couldn't.
jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet.
SA gets LWP ID via thread stack and funcs in libthread_db.so, but they
returns PIDs in container - they are different from host's PID. So I
added
the code to scan /proc//task to get all LWP IDs and they are kept
in a
Map in LinuxDebuggerLocal.
Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs
in
container. It helps SA to parse binaries in container.
This change has been pushed to submit repo, and it was failed on OS X
(mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963).
But I guess it causes JDK-8205906. This change affects to Linux only.
Could you review it?
Thanks,
Yasumasa
Re: PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
Hi Chris,
Thank you for your comment.
I uploaded new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.02/
I tested my change on Linux x64, but I cannot check it on other
platform (includes older Linux).
However SA tests are included in HotSpot tier 1 tests. Tests on submit
repo work fine with this change
(mach5-one-ysuenaga-JDK-8205992-20180720-0305-31840).
Thanks,
Yasumasa
2018-07-20 3:26 GMT+09:00 Chris Plummer :
> Hi Yasumasa,
>
> 84 // It maps the LWPID in the host to it in the container.
>
> "it" -> "the PID"
>
> 286 // Get LWPID in the host from the container's LWPID.
> 287 public int getHostPID(int id) {
> 288 try {
> 289 return nspidMap.get(id);
> 290 } catch (NullPointerException e) {
> 291 return -1;
> 292 }
> 293 }
>
> What is the source of the NPE here? Is it because nspidMap was never
> initialized because the process is not in a container? In that case I think
> you should be checking for null rather than having an NPE be part of normal
> execution.
>
> 42 int hostPID =
> ((LinuxDebuggerLocal)debugger).getHostPID(pid);
> 43 if (hostPID != -1) {
> 44 pid = hostPID;
> 45 }
>
> A comment here would be helpful.
>
> The rest looks good. I should probably run it through some internal testing.
> Let me know when you have a final webrev.
>
> thanks,
>
> Chris
>
>
> On 7/18/18 5:59 AM, Yasumasa Suenaga wrote:
>>
>> PING:
>>
>> Could you review it?
>>
>>JBS:https://bugs.openjdk.java.net/browse/JDK-8205992
>>webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/
>>
>> This change has been reviewed by Jini.
>> We need a Reviewer.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2018/07/12 13:42, Yasumasa Suenaga wrote:
>>>
>>> Thanks Jini,
>>>
>>> I uploaded new webrev. It contains some comments and removing extra
>>> space.
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/
>>>
>>>
>>> Yasumasa
>>>
>>>
>>>
>>> 2018-07-12 2:32 GMT+09:00 Jini George :
Hi Yasumasa,
This looks good to me except for one nit. And some more comments would
help.
For e.g., it would help to say that NSPidMap is to map the host to
container
lwpids.
The nit:
*
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html
Line 253: extra space after the parentheses
Thanks,
Jini.
On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote:
>
>
> PING: Could you review it?
>
>>JBS: https://bugs.openjdk.java.net/browse/JDK-8205992
>>webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/
>
>
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2018/06/28 22:12, Yasumasa Suenaga wrote:
>>
>>
>> Hi all,
>>
>> Please review this change.
>>
>>JBS: https://bugs.openjdk.java.net/browse/JDK-8205992
>>webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/
>>
>> I tried to attach jhsdb to java process in docker container from
>> container host, but it couldn't.
>> jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet.
>>
>> SA gets LWP ID via thread stack and funcs in libthread_db.so, but they
>> returns PIDs in container - they are different from host's PID. So I
>> added
>> the code to scan /proc//task to get all LWP IDs and they are kept
>> in a
>> Map in LinuxDebuggerLocal.
>>
>> Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs
>> in
>> container. It helps SA to parse binaries in container.
>>
>> This change has been pushed to submit repo, and it was failed on OS X
>> (mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963).
>> But I guess it causes JDK-8205906. This change affects to Linux only.
>>
>> Could you review it?
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>
>
Re: PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
Hi Yasumasa,
84 // It maps the LWPID in the host to it in the container.
"it" -> "the PID"
286 // Get LWPID in the host from the container's LWPID.
287 public int getHostPID(int id) {
288 try {
289 return nspidMap.get(id);
290 } catch (NullPointerException e) {
291 return -1;
292 }
293 }
What is the source of the NPE here? Is it because nspidMap was never
initialized because the process is not in a container? In that case I
think you should be checking for null rather than having an NPE be part
of normal execution.
42 int hostPID =
((LinuxDebuggerLocal)debugger).getHostPID(pid);
43 if (hostPID != -1) {
44 pid = hostPID;
45 }
A comment here would be helpful.
The rest looks good. I should probably run it through some internal
testing. Let me know when you have a final webrev.
thanks,
Chris
On 7/18/18 5:59 AM, Yasumasa Suenaga wrote:
PING:
Could you review it?
JBS: https://bugs.openjdk.java.net/browse/JDK-8205992
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/
This change has been reviewed by Jini.
We need a Reviewer.
Thanks,
Yasumasa
On 2018/07/12 13:42, Yasumasa Suenaga wrote:
Thanks Jini,
I uploaded new webrev. It contains some comments and removing extra
space.
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/
Yasumasa
2018-07-12 2:32 GMT+09:00 Jini George :
Hi Yasumasa,
This looks good to me except for one nit. And some more comments
would help.
For e.g., it would help to say that NSPidMap is to map the host to
container
lwpids.
The nit:
*
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html
Line 253: extra space after the parentheses
Thanks,
Jini.
On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote:
PING: Could you review it?
JBS: https://bugs.openjdk.java.net/browse/JDK-8205992
webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/
Thanks,
Yasumasa
On 2018/06/28 22:12, Yasumasa Suenaga wrote:
Hi all,
Please review this change.
JBS: https://bugs.openjdk.java.net/browse/JDK-8205992
webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/
I tried to attach jhsdb to java process in docker container from
container host, but it couldn't.
jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet.
SA gets LWP ID via thread stack and funcs in libthread_db.so, but
they
returns PIDs in container - they are different from host's PID. So
I added
the code to scan /proc//task to get all LWP IDs and they are
kept in a
Map in LinuxDebuggerLocal.
Also SA_ALTROOT is set to /proc//root if SA detects debuggee
runs in
container. It helps SA to parse binaries in container.
This change has been pushed to submit repo, and it was failed on OS X
(mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963).
But I guess it causes JDK-8205906. This change affects to Linux only.
Could you review it?
Thanks,
Yasumasa
PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
PING: Could you review it? JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/ This change has been reviewed by Jini. We need a Reviewer. Thanks, Yasumasa On 2018/07/12 13:42, Yasumasa Suenaga wrote: Thanks Jini, I uploaded new webrev. It contains some comments and removing extra space. http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/ Yasumasa 2018-07-12 2:32 GMT+09:00 Jini George : Hi Yasumasa, This looks good to me except for one nit. And some more comments would help. For e.g., it would help to say that NSPidMap is to map the host to container lwpids. The nit: * http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html Line 253: extra space after the parentheses Thanks, Jini. On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote: PING: Could you review it? JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ Thanks, Yasumasa On 2018/06/28 22:12, Yasumasa Suenaga wrote: Hi all, Please review this change. JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ I tried to attach jhsdb to java process in docker container from container host, but it couldn't. jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet. SA gets LWP ID via thread stack and funcs in libthread_db.so, but they returns PIDs in container - they are different from host's PID. So I added the code to scan /proc//task to get all LWP IDs and they are kept in a Map in LinuxDebuggerLocal. Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs in container. It helps SA to parse binaries in container. This change has been pushed to submit repo, and it was failed on OS X (mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963). But I guess it causes JDK-8205906. This change affects to Linux only. Could you review it? Thanks, Yasumasa
Re: PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
Thanks Jini! I'm waiting for Reviewer. Yasumasa 2018-07-12 14:09 GMT+09:00 Jini George : > Looks good to me. > > Thanks! > Jini (Not a Reviewer). > > > On 7/12/2018 10:12 AM, Yasumasa Suenaga wrote: >> >> Thanks Jini, >> >> I uploaded new webrev. It contains some comments and removing extra space. >> >> http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/ >> >> >> Yasumasa >> >> >> >> 2018-07-12 2:32 GMT+09:00 Jini George : >>> >>> Hi Yasumasa, >>> >>> This looks good to me except for one nit. And some more comments would >>> help. >>> For e.g., it would help to say that NSPidMap is to map the host to >>> container >>> lwpids. >>> >>> The nit: >>> >>> * >>> >>> http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html >>> Line 253: extra space after the parentheses >>> >>> Thanks, >>> Jini. >>> >>> On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote: PING: Could you review it? >JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 >webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ Thanks, Yasumasa On 2018/06/28 22:12, Yasumasa Suenaga wrote: > > > Hi all, > > Please review this change. > >JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 >webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ > > I tried to attach jhsdb to java process in docker container from > container host, but it couldn't. > jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet. > > SA gets LWP ID via thread stack and funcs in libthread_db.so, but they > returns PIDs in container - they are different from host's PID. So I > added > the code to scan /proc//task to get all LWP IDs and they are kept > in a > Map in LinuxDebuggerLocal. > > Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs > in > container. It helps SA to parse binaries in container. > > This change has been pushed to submit repo, and it was failed on OS X > (mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963). > But I guess it causes JDK-8205906. This change affects to Linux only. > > Could you review it? > > > Thanks, > > Yasumasa > >>> >
Re: PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
Looks good to me. Thanks! Jini (Not a Reviewer). On 7/12/2018 10:12 AM, Yasumasa Suenaga wrote: Thanks Jini, I uploaded new webrev. It contains some comments and removing extra space. http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/ Yasumasa 2018-07-12 2:32 GMT+09:00 Jini George : Hi Yasumasa, This looks good to me except for one nit. And some more comments would help. For e.g., it would help to say that NSPidMap is to map the host to container lwpids. The nit: * http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html Line 253: extra space after the parentheses Thanks, Jini. On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote: PING: Could you review it? JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ Thanks, Yasumasa On 2018/06/28 22:12, Yasumasa Suenaga wrote: Hi all, Please review this change. JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ I tried to attach jhsdb to java process in docker container from container host, but it couldn't. jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet. SA gets LWP ID via thread stack and funcs in libthread_db.so, but they returns PIDs in container - they are different from host's PID. So I added the code to scan /proc//task to get all LWP IDs and they are kept in a Map in LinuxDebuggerLocal. Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs in container. It helps SA to parse binaries in container. This change has been pushed to submit repo, and it was failed on OS X (mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963). But I guess it causes JDK-8205906. This change affects to Linux only. Could you review it? Thanks, Yasumasa
Re: PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
Thanks Jini, I uploaded new webrev. It contains some comments and removing extra space. http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.01/ Yasumasa 2018-07-12 2:32 GMT+09:00 Jini George : > Hi Yasumasa, > > This looks good to me except for one nit. And some more comments would help. > For e.g., it would help to say that NSPidMap is to map the host to container > lwpids. > > The nit: > > * > http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html > Line 253: extra space after the parentheses > > Thanks, > Jini. > > On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote: >> >> PING: Could you review it? >> >>> JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 >>> webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ >> >> >> >> Thanks, >> >> Yasumasa >> >> >> On 2018/06/28 22:12, Yasumasa Suenaga wrote: >>> >>> Hi all, >>> >>> Please review this change. >>> >>> JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 >>> webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ >>> >>> I tried to attach jhsdb to java process in docker container from >>> container host, but it couldn't. >>> jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet. >>> >>> SA gets LWP ID via thread stack and funcs in libthread_db.so, but they >>> returns PIDs in container - they are different from host's PID. So I added >>> the code to scan /proc//task to get all LWP IDs and they are kept in a >>> Map in LinuxDebuggerLocal. >>> >>> Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs in >>> container. It helps SA to parse binaries in container. >>> >>> This change has been pushed to submit repo, and it was failed on OS X >>> (mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963). >>> But I guess it causes JDK-8205906. This change affects to Linux only. >>> >>> Could you review it? >>> >>> >>> Thanks, >>> >>> Yasumasa >>> >
Re: PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
Hi Yasumasa, This looks good to me except for one nit. And some more comments would help. For e.g., it would help to say that NSPidMap is to map the host to container lwpids. The nit: * http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.c.sdiff.html Line 253: extra space after the parentheses Thanks, Jini. On 7/4/2018 4:34 AM, Yasumasa Suenaga wrote: PING: Could you review it? JBS: https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ Thanks, Yasumasa On 2018/06/28 22:12, Yasumasa Suenaga wrote: Hi all, Please review this change. JBS: https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ I tried to attach jhsdb to java process in docker container from container host, but it couldn't. jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet. SA gets LWP ID via thread stack and funcs in libthread_db.so, but they returns PIDs in container - they are different from host's PID. So I added the code to scan /proc//task to get all LWP IDs and they are kept in a Map in LinuxDebuggerLocal. Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs in container. It helps SA to parse binaries in container. This change has been pushed to submit repo, and it was failed on OS X (mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963). But I guess it causes JDK-8205906. This change affects to Linux only. Could you review it? Thanks, Yasumasa
PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers
PING: Could you review it? JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ Thanks, Yasumasa On 2018/06/28 22:12, Yasumasa Suenaga wrote: Hi all, Please review this change. JBS:https://bugs.openjdk.java.net/browse/JDK-8205992 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8205992/webrev.00/ I tried to attach jhsdb to java process in docker container from container host, but it couldn't. jcmd supports PID namespace in JDK-8193710, but jhsdb hasn't yet. SA gets LWP ID via thread stack and funcs in libthread_db.so, but they returns PIDs in container - they are different from host's PID. So I added the code to scan /proc//task to get all LWP IDs and they are kept in a Map in LinuxDebuggerLocal. Also SA_ALTROOT is set to /proc//root if SA detects debuggee runs in container. It helps SA to parse binaries in container. This change has been pushed to submit repo, and it was failed on OS X (mach5-one-ysuenaga-JDK-8205992-20180628-1015-28963). But I guess it causes JDK-8205906. This change affects to Linux only. Could you review it? Thanks, Yasumasa
