Codereview request: 8007710 runtime/7158988/FieldMonitor.java fails with com.sun.jdi.VMDisconnectedException: Connection closed

[Jaroslav Bachorik]

On 11.2.2014 17:17, shanliang wrote:
> Jaroslav Bachorik wrote:
>> On 11.2.2014 16:31, shanliang wrote:
>>> Staffan Larsen wrote:
>>>> Hi Shanliang,
>>>>
>>>> I can’t quite see how the test can fail in this way. When the
>>>> ClassPrepareEvent happens, the debuggee will be suspended. So when
>>>> addFieldWatch() is called, the debuggee should not have moved.
>>> I am not expert of jdi so I may miss something here. I checked the
>>> failure trace and saw the report exception happen when FieldMonitor
>>> received ClassPrepareEvent and was doing addFieldWatch. FieldMonitor did
>>> call "vm.resume()" before treating events.
>>
>> AFAICS, calling vm.resume() results in an almost immediate debuggee
>> death. The gc() invoking thread "d" is flagged as a deamon and as such
>> doesn't prevent the process from exiting. The other thread is not a
>> daemon but will finish in only few cycles.
> I looked at the class com.sun.jdi.VirtualMachine, here is the Javadoc of
> the method "resume":
>     /**
>      * Continues the execution of the application running in this
>      * virtual machine. All threads are resumed as documented in
>      * {@link ThreadReference#resume}.
>      *
>      * @throws VMCannotBeModifiedException if the VirtualMachine is
> read-only - see {@link VirtualMachine#canBeModified()}.
>      *
>      * @see #suspend
>      */
>     void resume();
> My understanding is that the debuggee resumes to work after this call,
> instead to die?
Yes. It resumes. But only for a few microseconds at best. Those 10 
string concatenations will not take long. So the VM will exit almost 
immediately after receiving the resume command.

>>
>>>
>>> I reproduced the bug by add sleep(1000) after vm.resume() but before
>>> calling eventQueue.remove();
>>
>> It looks like some kind of synchronization between the debugger and
>> the debuggee is necessary. But I wonder if you should better use the
>> process.getOuptuptStream() to write and flush a message for the
>> debugee indicating that it can exit. And in the debugee you would just
>> do System.in.read() as the last statement in the main() method. Seems
>> more robust than involving files.
> It could work, but creating a file in the testing directory should have
> no issue, but yes maybe less performance.
... and possible file system issues, necessity to clean up the file, not 
ready for parallelization etc. BTW, I have a recollection that you 
should create test specific files in a path specified by some other 
system property, not "user.dir". I can try to find the property name.

-JB-

>
> Thanks,
> Shanliang
>>
>> Cheers,
>>
>> -JB-
>>
>>>
>>> Thanks,
>>> Shanliang
>>>> One problem I do see with the test is that it does not wait for a
>>>> VMStartEvent before setting up requests. I’m not sure if that could
>>>> cause the failure in the bug report, though.
>>>>
>>>> /Staffan
>>>>
>>>> On 11 feb 2014, at 15:13, shanliang <shanliang.jiang at oracle.com> wrote:
>>>>
>>>>> Hi ,
>>>>>
>>>>> The problem could be that FieldMonitor did not have enough time to
>>>>> "addFieldWatch" but the vm to monitor (TestPostFieldModification) was
>>>>> already ended.
>>>>>
>>>>> So we should make sure that TestPostFieldModification exits after
>>>>> FieldMonitor has done necessary. The solution proposed here is that
>>>>> FieldMonitor creates a file after adding field watching, and
>>>>> TestPostFieldModification quits only after finding the file.
>>>>>
>>>>> web:
>>>>> http://icncweb.fr.oracle.com/~shjiang/webrev/8007710/00/
>>>>>
>>>>> bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8007710
>>>>>
>>>>> Thanks,
>>>>> Shanliang
>>>>
>>>
>>>
>>
>