RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

Robbin Ehn
Hi all, please review:

When starting the VM with agentlib and suspended, jcmd/jinfo/jstack/jmap don't work until a debugger have attach.

There is an underlying issue with AttachProvider that don't list that VM even if it's attachable.
That issue is present in JDK 8 (“jcmd -l” don’t list this process).

This fix only fixes the regression by not validating the pid against available VirtualMachineDescriptors.
One problem with not validating is that we will send SIGQUIT to that pid even if it’s not a java process, but it’s the same behavior as JDK 8.

The regression was introduced by https://bugs.openjdk.java.net/browse/JDK-8156537

VirtualMachineDescriptor was not easily create-able from just a string pid, you need an AttachProvider.
I instead change so we use a collection of pids where applicable.

Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
Issue: https://bugs.openjdk.java.net/browse/JDK-8176533

Passes jdk/test/sun/tools/ and internal test group.
Manual tested the regression case.

A new issue for the AttachProvider will be filed.

Thanks

/Robbin
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

serguei.spitsyn@oracle.com
Hi Robbin,

Nice fix.
Thank you for taking care about this issue so quickly!

One minor comment below.

http://cr.openjdk.java.net/~rehn/8176533/webrev/src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java.udiff.html

A typo in the comment: debugge-suspened => debuggee-suspened



On 3/15/17 10:41, Robbin Ehn wrote:

> Hi all, please review:
>
> When starting the VM with agentlib and suspended,
> jcmd/jinfo/jstack/jmap don't work until a debugger have attach.
>
> There is an underlying issue with AttachProvider that don't list that
> VM even if it's attachable.
> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>
> This fix only fixes the regression by not validating the pid against
> available VirtualMachineDescriptors.
> One problem with not validating is that we will send SIGQUIT to that
> pid even if it’s not a java process, but it’s the same behavior as JDK 8.
>
> The regression was introduced by
> https://bugs.openjdk.java.net/browse/JDK-8156537
>
> VirtualMachineDescriptor was not easily create-able from just a string
> pid, you need an AttachProvider.
> I instead change so we use a collection of pids where applicable.
>
> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>
> Passes jdk/test/sun/tools/ and internal test group.
> Manual tested the regression case.
>
> A new issue for the AttachProvider will be filed.

I've filed this one:
   https://bugs.openjdk.java.net/browse/JDK-8176828
    jtools do not list VM process launched with the debugger option
suspend=y

Feel free to update it if necessary.

Thanks,
Serguei

>
> Thanks
>
> /Robbin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

Dmitry Samersoff
In reply to this post by Robbin Ehn
Robbin,

Looks good to me.

-Dmitry

On 2017-03-15 20:41, Robbin Ehn wrote:

> Hi all, please review:
>
> When starting the VM with agentlib and suspended, jcmd/jinfo/jstack/jmap
> don't work until a debugger have attach.
>
> There is an underlying issue with AttachProvider that don't list that VM
> even if it's attachable.
> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>
> This fix only fixes the regression by not validating the pid against
> available VirtualMachineDescriptors.
> One problem with not validating is that we will send SIGQUIT to that pid
> even if it’s not a java process, but it’s the same behavior as JDK 8.
>
> The regression was introduced by
> https://bugs.openjdk.java.net/browse/JDK-8156537
>
> VirtualMachineDescriptor was not easily create-able from just a string
> pid, you need an AttachProvider.
> I instead change so we use a collection of pids where applicable.
>
> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>
> Passes jdk/test/sun/tools/ and internal test group.
> Manual tested the regression case.
>
> A new issue for the AttachProvider will be filed.
>
> Thanks
>
> /Robbin


--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

David Holmes
In reply to this post by serguei.spitsyn@oracle.com
On 16/03/2017 4:14 AM, [hidden email] wrote:

> Hi Robbin,
>
> Nice fix.
> Thank you for taking care about this issue so quickly!
>
> One minor comment below.
>
> http://cr.openjdk.java.net/~rehn/8176533/webrev/src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java.udiff.html
>
>
> A typo in the comment: debugge-suspened => debuggee-suspened

typo: suspened -> suspended

Not a review :)

David

>
>
> On 3/15/17 10:41, Robbin Ehn wrote:
>> Hi all, please review:
>>
>> When starting the VM with agentlib and suspended,
>> jcmd/jinfo/jstack/jmap don't work until a debugger have attach.
>>
>> There is an underlying issue with AttachProvider that don't list that
>> VM even if it's attachable.
>> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>>
>> This fix only fixes the regression by not validating the pid against
>> available VirtualMachineDescriptors.
>> One problem with not validating is that we will send SIGQUIT to that
>> pid even if it’s not a java process, but it’s the same behavior as JDK 8.
>>
>> The regression was introduced by
>> https://bugs.openjdk.java.net/browse/JDK-8156537
>>
>> VirtualMachineDescriptor was not easily create-able from just a string
>> pid, you need an AttachProvider.
>> I instead change so we use a collection of pids where applicable.
>>
>> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>>
>> Passes jdk/test/sun/tools/ and internal test group.
>> Manual tested the regression case.
>>
>> A new issue for the AttachProvider will be filed.
>
> I've filed this one:
>   https://bugs.openjdk.java.net/browse/JDK-8176828
>    jtools do not list VM process launched with the debugger option
> suspend=y
>
> Feel free to update it if necessary.
>
> Thanks,
> Serguei
>
>>
>> Thanks
>>
>> /Robbin
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

serguei.spitsyn@oracle.com
On 3/15/17 14:23, David Holmes wrote:

> On 16/03/2017 4:14 AM, [hidden email] wrote:
>> Hi Robbin,
>>
>> Nice fix.
>> Thank you for taking care about this issue so quickly!
>>
>> One minor comment below.
>>
>> http://cr.openjdk.java.net/~rehn/8176533/webrev/src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java.udiff.html 
>>
>>
>>
>> A typo in the comment: debugge-suspened => debuggee-suspened
>
> typo: suspened -> suspended

It is funny, I've overlooked it. :)

Thanks,
Serguei

>
> Not a review :)
>
> David
>
>>
>>
>> On 3/15/17 10:41, Robbin Ehn wrote:
>>> Hi all, please review:
>>>
>>> When starting the VM with agentlib and suspended,
>>> jcmd/jinfo/jstack/jmap don't work until a debugger have attach.
>>>
>>> There is an underlying issue with AttachProvider that don't list that
>>> VM even if it's attachable.
>>> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>>>
>>> This fix only fixes the regression by not validating the pid against
>>> available VirtualMachineDescriptors.
>>> One problem with not validating is that we will send SIGQUIT to that
>>> pid even if it’s not a java process, but it’s the same behavior as
>>> JDK 8.
>>>
>>> The regression was introduced by
>>> https://bugs.openjdk.java.net/browse/JDK-8156537
>>>
>>> VirtualMachineDescriptor was not easily create-able from just a string
>>> pid, you need an AttachProvider.
>>> I instead change so we use a collection of pids where applicable.
>>>
>>> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>>>
>>> Passes jdk/test/sun/tools/ and internal test group.
>>> Manual tested the regression case.
>>>
>>> A new issue for the AttachProvider will be filed.
>>
>> I've filed this one:
>>   https://bugs.openjdk.java.net/browse/JDK-8176828
>>    jtools do not list VM process launched with the debugger option
>> suspend=y
>>
>> Feel free to update it if necessary.
>>
>> Thanks,
>> Serguei
>>
>>>
>>> Thanks
>>>
>>> /Robbin
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

serguei.spitsyn@oracle.com
In reply to this post by Dmitry Samersoff
Hi Robbin,

You have two reviews now, so it is Ok to push assuming the testing is good.
Please, let us know if anything else is needed.

It seems, today is the last day before an approval will be required for
a push.

Thanks,
Serguei



On 3/15/17 11:14, Dmitry Samersoff wrote:

> Robbin,
>
> Looks good to me.
>
> -Dmitry
>
> On 2017-03-15 20:41, Robbin Ehn wrote:
>> Hi all, please review:
>>
>> When starting the VM with agentlib and suspended, jcmd/jinfo/jstack/jmap
>> don't work until a debugger have attach.
>>
>> There is an underlying issue with AttachProvider that don't list that VM
>> even if it's attachable.
>> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>>
>> This fix only fixes the regression by not validating the pid against
>> available VirtualMachineDescriptors.
>> One problem with not validating is that we will send SIGQUIT to that pid
>> even if it’s not a java process, but it’s the same behavior as JDK 8.
>>
>> The regression was introduced by
>> https://bugs.openjdk.java.net/browse/JDK-8156537
>>
>> VirtualMachineDescriptor was not easily create-able from just a string
>> pid, you need an AttachProvider.
>> I instead change so we use a collection of pids where applicable.
>>
>> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>>
>> Passes jdk/test/sun/tools/ and internal test group.
>> Manual tested the regression case.
>>
>> A new issue for the AttachProvider will be filed.
>>
>> Thanks
>>
>> /Robbin
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

Robbin Ehn
In reply to this post by serguei.spitsyn@oracle.com
Thanks Serguei!

On 03/15/2017 07:14 PM, [hidden email] wrote:

> Hi Robbin,
>
> Nice fix.
> Thank you for taking care about this issue so quickly!
>
> One minor comment below.
>
> http://cr.openjdk.java.net/~rehn/8176533/webrev/src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java.udiff.html
>
> A typo in the comment: debugge-suspened => debuggee-suspened

Yes.

/Robbin

>
>
>
> On 3/15/17 10:41, Robbin Ehn wrote:
>> Hi all, please review:
>>
>> When starting the VM with agentlib and suspended, jcmd/jinfo/jstack/jmap don't work until a debugger have attach.
>>
>> There is an underlying issue with AttachProvider that don't list that VM even if it's attachable.
>> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>>
>> This fix only fixes the regression by not validating the pid against available VirtualMachineDescriptors.
>> One problem with not validating is that we will send SIGQUIT to that pid even if it’s not a java process, but it’s the same behavior as JDK 8.
>>
>> The regression was introduced by https://bugs.openjdk.java.net/browse/JDK-8156537
>>
>> VirtualMachineDescriptor was not easily create-able from just a string pid, you need an AttachProvider.
>> I instead change so we use a collection of pids where applicable.
>>
>> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>>
>> Passes jdk/test/sun/tools/ and internal test group.
>> Manual tested the regression case.
>>
>> A new issue for the AttachProvider will be filed.
>
> I've filed this one:
>   https://bugs.openjdk.java.net/browse/JDK-8176828
>    jtools do not list VM process launched with the debugger option suspend=y
>
> Feel free to update it if necessary.
>
> Thanks,
> Serguei
>
>>
>> Thanks
>>
>> /Robbin
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

Robbin Ehn
In reply to this post by Dmitry Samersoff
Thanks for looking at this so quickly Dmitry!

/Robbin

On 03/15/2017 07:14 PM, Dmitry Samersoff wrote:

> Robbin,
>
> Looks good to me.
>
> -Dmitry
>
> On 2017-03-15 20:41, Robbin Ehn wrote:
>> Hi all, please review:
>>
>> When starting the VM with agentlib and suspended, jcmd/jinfo/jstack/jmap
>> don't work until a debugger have attach.
>>
>> There is an underlying issue with AttachProvider that don't list that VM
>> even if it's attachable.
>> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>>
>> This fix only fixes the regression by not validating the pid against
>> available VirtualMachineDescriptors.
>> One problem with not validating is that we will send SIGQUIT to that pid
>> even if it’s not a java process, but it’s the same behavior as JDK 8.
>>
>> The regression was introduced by
>> https://bugs.openjdk.java.net/browse/JDK-8156537
>>
>> VirtualMachineDescriptor was not easily create-able from just a string
>> pid, you need an AttachProvider.
>> I instead change so we use a collection of pids where applicable.
>>
>> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>>
>> Passes jdk/test/sun/tools/ and internal test group.
>> Manual tested the regression case.
>>
>> A new issue for the AttachProvider will be filed.
>>
>> Thanks
>>
>> /Robbin
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

Robbin Ehn
In reply to this post by David Holmes
Hi David,

On 03/15/2017 10:23 PM, David Holmes wrote:

> On 16/03/2017 4:14 AM, [hidden email] wrote:
>> Hi Robbin,
>>
>> Nice fix.
>> Thank you for taking care about this issue so quickly!
>>
>> One minor comment below.
>>
>> http://cr.openjdk.java.net/~rehn/8176533/webrev/src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java.udiff.html
>>
>>
>> A typo in the comment: debugge-suspened => debuggee-suspened
>
> typo: suspened -> suspended

:)

/Robbin

>
> Not a review :)
>
> David
>
>>
>>
>> On 3/15/17 10:41, Robbin Ehn wrote:
>>> Hi all, please review:
>>>
>>> When starting the VM with agentlib and suspended,
>>> jcmd/jinfo/jstack/jmap don't work until a debugger have attach.
>>>
>>> There is an underlying issue with AttachProvider that don't list that
>>> VM even if it's attachable.
>>> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>>>
>>> This fix only fixes the regression by not validating the pid against
>>> available VirtualMachineDescriptors.
>>> One problem with not validating is that we will send SIGQUIT to that
>>> pid even if it’s not a java process, but it’s the same behavior as JDK 8.
>>>
>>> The regression was introduced by
>>> https://bugs.openjdk.java.net/browse/JDK-8156537
>>>
>>> VirtualMachineDescriptor was not easily create-able from just a string
>>> pid, you need an AttachProvider.
>>> I instead change so we use a collection of pids where applicable.
>>>
>>> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>>>
>>> Passes jdk/test/sun/tools/ and internal test group.
>>> Manual tested the regression case.
>>>
>>> A new issue for the AttachProvider will be filed.
>>
>> I've filed this one:
>>   https://bugs.openjdk.java.net/browse/JDK-8176828
>>    jtools do not list VM process launched with the debugger option
>> suspend=y
>>
>> Feel free to update it if necessary.
>>
>> Thanks,
>> Serguei
>>
>>>
>>> Thanks
>>>
>>> /Robbin
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8176533: REGRESSION: a java process is not recognized by jcmd/jinfo/jstack/jmap tool

Robbin Ehn
In reply to this post by serguei.spitsyn@oracle.com
Hi Serguei,

On 03/16/2017 05:05 AM, [hidden email] wrote:
> Hi Robbin,
>
> You have two reviews now, so it is Ok to push assuming the testing is good.
> Please, let us know if anything else is needed.

Yes thanks!

>
> It seems, today is the last day before an approval will be required for a push.

Will push today!

/Robbin

>
> Thanks,
> Serguei
>
>
>
> On 3/15/17 11:14, Dmitry Samersoff wrote:
>> Robbin,
>>
>> Looks good to me.
>>
>> -Dmitry
>>
>> On 2017-03-15 20:41, Robbin Ehn wrote:
>>> Hi all, please review:
>>>
>>> When starting the VM with agentlib and suspended, jcmd/jinfo/jstack/jmap
>>> don't work until a debugger have attach.
>>>
>>> There is an underlying issue with AttachProvider that don't list that VM
>>> even if it's attachable.
>>> That issue is present in JDK 8 (“jcmd -l” don’t list this process).
>>>
>>> This fix only fixes the regression by not validating the pid against
>>> available VirtualMachineDescriptors.
>>> One problem with not validating is that we will send SIGQUIT to that pid
>>> even if it’s not a java process, but it’s the same behavior as JDK 8.
>>>
>>> The regression was introduced by
>>> https://bugs.openjdk.java.net/browse/JDK-8156537
>>>
>>> VirtualMachineDescriptor was not easily create-able from just a string
>>> pid, you need an AttachProvider.
>>> I instead change so we use a collection of pids where applicable.
>>>
>>> Webrev: http://cr.openjdk.java.net/~rehn/8176533/webrev/
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8176533
>>>
>>> Passes jdk/test/sun/tools/ and internal test group.
>>> Manual tested the regression case.
>>>
>>> A new issue for the AttachProvider will be filed.
>>>
>>> Thanks
>>>
>>> /Robbin
>>
>
Loading...