The updated webrev looks good to me Ivan. Sorry for sending you off on
the wrong track!
Let me know if you need a sponsor to push this.
-Chris.
On 06/13/2013 11:33 AM, Ivan Gerasimov wrote:
Thank you, David.
I've reverted the code back, added volatile specification to the count
variable.
Would you please review the webrev with this only change (comparing to
webrev.0):
http://cr.openjdk.java.net/~igerasim/7181748/webrev.2/
Sincerely yours,
Ivan
On 12.06.2013 13:49, David Holmes wrote:
On 12/06/2013 12:13 AM, Ivan Gerasimov wrote:
Chris and David, thanks for review.
I've updated the test so the threads use CountDownLatch to wait for each
other.
Now, instead of checking whether the 'second' thread has incremented a
count, the 'main' thread waits for it to call countDown().
If a timeout was elapsed before the call, I assume that the 'second'
thread is suspended and the test passes.
I do not advise doing this. The second thread can be suspended inside
the CountdownLatch code. When suspend is involved you do not want any
direct interaction between the main thread and the thread being
suspended.
David
-----
Could you please review the updated webrev:
http://cr.openjdk.java.net/~igerasim/7181748/webrev.1/
Sincerely,
Ivan
On 11.06.2013 13:37, Chris Hegarty wrote:
On 06/11/2013 08:08 AM, David Holmes wrote:
On 11/06/2013 1:54 AM, Chris Hegarty wrote:
I'm not sure I ever saw this test fail, but it does look like it has
issues.
I would much prefer to see a j.u.c.Latch/Phaser used to synchronize
across these threads.
I don't think that is possible. The main thread wants to reset the
count
after the suspend has been issued by "first", but once the suspend has
been issued the "first" thread can't signal the synchronizer to let
the
main thread proceed. The poll around isAlive() does address the main
issue of a slowly starting thread.
Ah yes, thanks David. In which case I'm fine with the change.
For completeness the count variable should be volatile as well.
Agreed.
-Chris.
Cheers,
David
-Chris.
On 10/06/2013 13:51, Ivan Gerasimov wrote:
Hello everyone!
The test of ThreadGroup.Suspend() was reported to fail on rare
occasions.
It can happen on a busy machine that 1 second delay would not be
enough
for the second thread to start.
Then the first thread would suspend only itself and the test would
fail.
Earlier, another test was updated for similar reasons [1], [2].
The proposed test can still report false *positive* results if the
second thread has had no chance to execute during one second
after it
had started.
To avoid them there must be a way do distinguish suspended threads.
WEBREV: http://cr.openjdk.java.net/~igerasim/7181748/webrev.0/
BUG: http://bugs.sun.com/view_bug.do?bug_id=7181748
[1] http://bugs.sun.com/view_bug.do?bug_id=7084033
[2] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/184578f3e8b9
Sincerely yours,
Ivan Gerasimov
<https://jbs.oracle.com/bugs/browse/JDK-7084033>
