RFR: 8225690: Multiple AttachListener threads can be created

[Yasumasa Suenaga]

Hi Chris,

I uploaded new webrev:

   http://cr.openjdk.java.net/~ysuenaga/JDK-8225690/webrev.03/

On 2019/07/15 15:48, Chris Plummer wrote:

...snip...

>>> I don't understand the need for the following in attach_listener_thread_entry()
>>>
>>>   428 AttachListener::set_state(AL_NOT_INITIALIZED);
>>>
>>> There is no path to this statement since the only way out of the loop is the following:
>>
>> I agree with you, but I think we need to reset the status of Attach Listener
>> in the end of the function.
> Why, if it is never executed?
>>
>> We might be able to use ShouldNotReachHere() at here.
>>
> I think ShouldNotReachHere() would be appropriate at the end of this function, but it's really just adding an assert for what I've already said is the case. You can't reach this point so there is no reason to add any logic there (other than asserting).

I replaced it to ShouldNotReachHere() in new webrev.


...snip...


>> If you mean the failure of init() and/or is_init_trigger(),
>> I do not have idea how to test it.
> Yes, I mean unexpected failures that you've written code to handle. The best way to test them is either in gdb or just programmatically introduce the failure (change the code so the failure happens). I'm just asking that you step through the code one time under failure, not that you have a test to induce the failure, which probably is not possible.

I tested this patch with the change to return false immediately in is_init_trigger().
It works fine with ConcAttachTest.java . LingeredApp did not create Attach Listener thread, and also it did not hang.


Thanks,

Yasumasa