Jaroslav Bachorik wrote:
Hi,
On 12/12/2014 09:56 AM, shanliang wrote:
Daniel Fuchs wrote:
Hi Shanliang,
These two statements are no longer needed and should be removed - as
they
are misleading:
64 if (!bb.gotLock) {
65 throw new RuntimeException("Failed to get lock,
impossible!");
66 }
81 if (!wb.done) {
82 throw new RuntimeException("It is blocked!");
83 }
Yes, can be removed.
which makes me wonder whether your changes are deeper than intended.
Now the test will either succeed, or fail in jtreg timeout. The
condition
"It is blocked!"
can no longer be detected by the test. Is that your intention? If
that's
your intention - I guess it is OK - but maybe it would deserve a
comment
in the @summary of the test. Something like 'This bug attempts to
reproduce
a deadlock situation and will block forever if the deadlock is
present.'
Updated.
Here is the new version:
http://cr.openjdk.java.net/~sjiang/JDK-8067241/01/
I know that I'm starting to repeat myself but couldn't you replace
L58-61 and L91-95 with a simple Phaser?
Like
```
L58-61: Phaser p = new Phaser(2); p.arriveAndAwaitAdvance()
Then pass the Phaser instance to BadBoy and
L91-95: p.arriveAndAwaitAdvance();
```
This way the main thread is blocked till the BadBoy thread has
acquired the BadBoy's lock and the code is more concise.
I agree Daniel's comment that we may need more investigation for using
Phaser, Phaser might be a good solution but the current synchronization
solution should work too and I have ran the test on jprt.
So let's keep the current solution and investivating on Phaser if the
test fails again.
Shanliang
-JB-
Thanks,
Shanliang
best regards,
-- daniel
On 12/12/14 8:33 AM, shanliang wrote:
Hi,
It is a test bug, it is not correct:
while(!wb.done || timeToWait > 0) {
it should be:
while(!wb.done && timeToWait > 0) {
|| should be changed to &&
Another issue is that the waiting time could be not enough (final
long timeout = 2000).
The fix is to remove the waiting time specified in the test, the
timeout of test harness will be used as the max waiting time.
bug: https://bugs.openjdk.java.net/browse/JDK-8067241
webrev: http://cr.openjdk.java.net/~sjiang/JDK-8067241/00/
thanks,
Shanliang
