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
