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

[shanliang]

Here is the new fix in which FieldMonitor will write to 
TestPostFieldModification, to inform the latter to quit, as suggested bu 
Jaroslav
    http://cr.openjdk.java.net/~sjiang/JDK-8007710/01/

Thanks,
Shanliang

shanliang wrote:
> 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
>>>>>
>>>>
>>>>
>>>
>>
>