RFR: 8178784: Revisit changes for making Serviceability Agent attach functionality optional
Jini George
jini.george at oracle.com
Wed May 31 13:26:34 UTC 2017
Thanks, Mikael. I will be porting the changes to portola/jdk9 too. At
this point, I think we should be able to replace the thread_db logic
with the iteration over /proc. I have filed
https://bugs.openjdk.java.net/browse/JDK-8181313 to look further into this.
Thanks,
Jini.
On 5/30/2017 11:37 PM, Mikael Vidstedt wrote:
> Looks great, thanks for making the changes! Once you have this pushed can you please back port the change to portola/jdk9 as well?
>
>
> One last question just for my understanding: how "portable” do you think the new code is? Not because we necessarily want to actually do so, but in theory, would it be possible to make the new code the default and remove the thread_db code altogether?
>
> Cheers,
> Mikael
>
>> On May 29, 2017, at 1:30 AM, Jini George <jini.george at oracle.com> wrote:
>>
>> Thanks much for the review, Mikael.
>>
>> The modified webrev is at: http://cr.openjdk.java.net/~jgeorge/8178784/webrev.01/index.html
>>
>> I have shifted the logic of reading in the /proc/<pid>/task/ entries to read_thread_info in libproc_impl.c, and ps_core.c is more or less intact except for the strerror_r related correction. I have removed the extra spaces between the function name and the parenthesis also.
>>
>> Thanks,
>> Jini.
>>
>> On 5/25/2017 11:57 PM, Mikael Vidstedt wrote:
>>> Jini,
>>>
>>> Thanks a lot for looking into this! The changes look good. One question:
>>>
>>> Would it be possible to implement the whole opendir/readdir/closedir walk as an alternative implementation of read_thread_info in libproc_impl.c instead? Without having tried it, I think that would mean you can revert the whole ps_core.c file back to what it was before I added the #include guards, and all the special logic will instead be in libproc_impl.c.
>>>
>>> Minor nit: there are a few places where you have a space between the function name and the parenthesis (after readdir for example), which doesn’t seem to be the style used in the rest of the places in the SA.
>>>
>>> Cheers,
>>> Mikael
>>>
>>>> On May 25, 2017, at 5:26 AM, Jini George <jini.george at oracle.com> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Please review the changes for enabling live debugging (attaching to a process) with SA on Alpine Linux.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8178784/webrev.00/
>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8178784
>>>>
>>>> The crux of the changes are:
>>>>
>>>> 1. To include the functions #ifdef-ed out by INCLUDE_SA_ATTACH, and to correct build errors resulting due to the inclusion of these functions.
>>>> 2. SA on Linux uses the thread_db library for attach debugging. This library is not available on Alpine Linux. So instead of relying on thread_db, I have made changes to iterate over the list of tasks in the /proc/<pid>/task directory to find the list of threads to ptrace attach to. This enables the jhsdb commands like threads, jstack etc.
>>>>
>>>> Thanks,
>>>> Jini.
>>>>
>>>>
More information about the portola-dev
mailing list