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

[shanliang]

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?
In fact the problem is here, the vm (TestPostFieldModification) should 
not die before FieldMonitor finishes addFieldWatch.

Shanliang
>>
>>>
>>> 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.
>
> 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
>>>>
>>>
>>>
>>
>