PING: RFR: 8205992: jhsdb cannot attach to Java processes running in Docker containers

[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 <jini.george at oracle.com>:
>>> 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/<PID>/task to get all LWP IDs and they are 
>>>>> kept in a
>>>>> Map in LinuxDebuggerLocal.
>>>>>
>>>>> Also SA_ALTROOT is set to /proc/<PID>/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
>>>>>
>>>