Bug ID: JDK-6498581 ThreadInterruptTest3 produces wrong output on Windows

JDK-6498581 : ThreadInterruptTest3 produces wrong output on Windows

Type: Bug
Component: hotspot
Sub-Component: runtime
Affected Version: 6u1,7

Priority: P3
Status: Closed
Resolution: Fixed
OS: windows_2000
CPU: x86

Submitted: 2006-11-29
Updated: 2019-10-15
Resolved: 2014-02-27

Versions (Unresolved/Resolved/Fixed)

The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.

JDK 8	JDK 9
8u20Fixed	9 b06Fixed

Related Reports

Relates :	JDK-6313903 - Thread.sleep(3) might wake up immediately on windows
Relates :	JDK-8036102 - part of the fix for 6498581 lost in mismerge
Relates :	JDK-6741489 - java/util/concurrent/FutureTask/BlockingTaskExecutor.java fails with java.lang.Error: Executor stuck
Relates :	JDK-6546236 - Thread interrupt() of Thread.sleep() can be lost on Solaris due to race with signal handler
Relates :	JDK-6535709 - interrupt of wait()ing thread isn't triggering InterruptedException - test intwait3

Description

While analysing PIT fail results.

Logs and detailes: 

/net/gtee.sfbay/export/gtee/results/MUSTANG/NIGHTLY/VM-MAIN/2006-11-24/Main_Baseline/vm/WINDOWS-I586/client/mixed/vm-vm_6.0_client_mixed_WINDOWS-I5862006-11-24-19-54-36

http://gtee.sfbay/gtee/results/MUSTANG/NIGHTLY/VM-MAIN/2006-11-24/Main_Baseline/vm/WINDOWS-I586/client/mixed/vm-vm_6.0_client_mixed_WINDOWS-I5862006-11-24-19-54-36/analysis.html

HOSTNAME=VM-X86-3



It looks like some of join() calls was not invoked (look for **++** in *.diff) and not syncronized.

There is the same bug with -server mode:

Details:

/net/gtee.sfbay/export/gtee/results/MUSTANG/NIGHTLY/VM-MAIN/2006-11-24/Main_Baseline/vm/WINDOWS-I586/server/mixed/vm-vm_6.0_server_mixed_WINDOWS-I5862006-11-24-19-48-11

http://gtee.sfbay/gtee/results/MUSTANG/NIGHTLY/VM-MAIN/2006-11-24/Main_Baseline/vm/WINDOWS-I586/server/mixed/vm-vm_6.0_server_mixed_WINDOWS-I5862006-11-24-19-48-11/analysis.html
HOSTNAME=KARISHMA
The test re-surfaced again in the U1 testing & here is the results file

http://gtee.sfbay.sun.com/gtee/results/MUSTANG_UR/PROMOTION/VM-WEEKLY/1.6.0_01-b04-070215143730/vm/64BITWIN03-AMD64/server/batch/vm-vm_6.0_server_batch_64BITWIN03-AMD642007-02-15-14-38-51/analysis.html#New_Failures

I wasnt able to reproduce it always but out of 4 turns I was able to reproduce it once.

Here is the log :
#
[2007-02-15T23:50:20.34] export CLASSPATH
[2007-02-15T23:50:20.34] export RAS_OPTIONS
[2007-02-15T23:50:20.34] export PATH
[2007-02-15T23:50:20.36] export TESTBASE
[2007-02-15T23:50:20.36] export SHELL
[2007-02-15T23:50:20.36] export WINDIR
[2007-02-15T23:50:20.36] export SystemRoot
[2007-02-15T23:50:20.36] 
[2007-02-15T23:50:20.36] 
[2007-02-15T23:50:20.36] # Dtonga sync function sub.
[2007-02-15T23:50:20.36] cwd=`echo $0 | sed 's/[^\/]*.tlog//'`
[2007-02-15T23:50:20.36] 
[2007-02-15T23:50:20.36] dt_wait () {
[2007-02-15T23:50:20.36] # Is not emplemented yet
[2007-02-15T23:50:20.36] 	sleep 5
[2007-02-15T23:50:20.36] }
[2007-02-15T23:50:20.36] dt_post () {
[2007-02-15T23:50:20.36] # Is not emplemented yet
[2007-02-15T23:50:20.36] 	echo $@
[2007-02-15T23:50:20.36] }
[2007-02-15T23:50:20.36] 
[2007-02-15T23:50:20.36] dt_kill () {
[2007-02-15T23:50:20.36] # Is not emplemented yet
[2007-02-15T23:50:20.36] 	echo $@
[2007-02-15T23:50:20.36] }
[2007-02-15T23:50:20.36] 
[2007-02-15T23:50:20.36] 
[2007-02-15T23:50:20.36] # Dtonga deploy function sub.
[2007-02-15T23:50:20.36] deploy () {
[2007-02-15T23:50:20.36] 	rsh $1 sh -s << --EOF--
[2007-02-15T23:50:20.36] 	`cat ${cwd}/${2}/${3}.tlog`
[2007-02-15T23:50:20.36] --EOF--
[2007-02-15T23:50:20.36] }
[2007-02-15T23:50:20.36] 
[2007-02-15T23:50:20.36] TEST_DEST_DIR="ThreadInterruptTest3"
[2007-02-15T23:50:40.69] # Actual: TEST_DEST_DIR=ThreadInterruptTest3
[2007-02-15T23:50:40.69] TESTNAME="${test_case_name}"
[2007-02-15T23:50:40.69] # Actual: TESTNAME=ThreadInterruptTest3
[2007-02-15T23:50:40.69] testName="runtime/threads//ThreadInterruptTest3"
[2007-02-15T23:50:40.69] # Actual: testName=runtime/threads//ThreadInterruptTest3
[2007-02-15T23:50:40.69] TESTDIR="${test_work_dir}"
[2007-02-15T23:50:40.69] # Actual: TESTDIR=X:/gtee/results/MUSTANG_UR/PROMOTION/VM-WEEKLY/1.6.0_01-b04-070215143730/vm/64BITWIN03-AMD64/server/batch/vm-vm_6.0_server_batch_64BITWIN03-AMD642007-02-15-14-38-51/ResultDir/ThreadInterruptTest3
[2007-02-15T23:50:40.69] testWorkDir="${test_work_dir}/"
[2007-02-15T23:50:40.69] # Actual: testWorkDir=X:/gtee/results/MUSTANG_UR/PROMOTION/VM-WEEKLY/1.6.0_01-b04-070215143730/vm/64BITWIN03-AMD64/server/batch/vm-vm_6.0_server_batch_64BITWIN03-AMD642007-02-15-14-38-51/ResultDir/ThreadInterruptTest3/
[2007-02-15T23:50:40.69] export testWorkDir
[2007-02-15T23:50:40.69] tlogOutFile="${test_work_dir}/${test_name}.tlog"
[2007-02-15T23:50:40.69] # Actual: tlogOutFile=X:/gtee/results/MUSTANG_UR/PROMOTION/VM-WEEKLY/1.6.0_01-b04-070215143730/vm/64BITWIN03-AMD64/server/batch/vm-vm_6.0_server_batch_64BITWIN03-AMD642007-02-15-14-38-51/ResultDir/ThreadInterruptTest3/ThreadInterruptTest3.tlog
[2007-02-15T23:50:40.69] testErrFile="${test_work_dir}/${test_name}.err"
[2007-02-15T23:50:40.69] # Actual: testErrFile=X:/gtee/results/MUSTANG_UR/PROMOTION/VM-WEEKLY/1.6.0_01-b04-070215143730/vm/64BITWIN03-AMD64/server/batch/vm-vm_6.0_server_batch_64BITWIN03-AMD642007-02-15-14-38-51/ResultDir/ThreadInterruptTest3/ThreadInterruptTest3.err
[2007-02-15T23:50:40.69] EXECUTE_CLASS="${test_name}"
[2007-02-15T23:50:40.69] # Actual: EXECUTE_CLASS=ThreadInterruptTest3
[2007-02-15T23:50:40.69] TONGA_FILTER="grep -v VM option"
[2007-02-15T23:50:40.69] # Actual: TONGA_FILTER=grep -v "VM option"
[2007-02-15T23:50:40.69] EXECUTE_CLASS="runtime.threads.ThreadInterruptTest3.ThreadInterruptTest3"
[2007-02-15T23:50:40.69] # Actual: EXECUTE_CLASS=runtime.threads.ThreadInterruptTest3.ThreadInterruptTest3
[2007-02-15T23:50:40.69] APPLICATION_TIMEOUT="${TIMEOUT}"
[2007-02-15T23:50:40.69] # Actual: APPLICATION_TIMEOUT=30
[2007-02-15T23:50:40.69] CLASSPATH="${test_work_dir}${PS}${CLASSPATH}"
[2007-02-15T23:50:40.69] # Actual: CLASSPATH=X:/gtee/results/MUSTANG_UR/PROMOTION/VM-WEEKLY/1.6.0_01-b04-070215143730/vm/64BITWIN03-AMD64/server/batch/vm-vm_6.0_server_batch_64BITWIN03-AMD642007-02-15-14-38-51/ResultDir/ThreadInterruptTest3;c:/temp/testbase/vm/bin/classes;c:/temp/Work/JDK/vm/windows-amd64/lib/tools.jar
[2007-02-15T23:50:40.69] export CLASSPATH
[2007-02-15T23:50:40.69] PATH="${PATH}${PS}${LD_LIBRARY_PATH}"
[2007-02-15T23:50:40.69] # Actual: PATH="c:/temp/Work/JDK/vm/windows-amd64/bin;C:/WINDOWS/system32;C:/MKS/mksnt;C:/Program Files (x86)/MKS Toolkit/mksnt;D:/Program Files (x86)/MKS Toolkit/mksnt;D:/MKS/mksnt;c:/temp/Work/JDK/vm/windows-amd64/jre/bin/server;c:/temp/Work/JDK/vm/windows-amd64/jre/bin;"
[2007-02-15T23:50:40.69] export PATH
[2007-02-15T23:50:40.69] ${JAVA}  ${JAVA_OPTS} ${EXECUTE_CLASS} ${TEST_ARGS} 1>${test_work_dir}/${test_name}.eout 
[2007-02-15T23:50:40.73] # Actual: c:/temp/Work/JDK/vm/windows-amd64/bin/java -server -Xbatch -XX:-ShowMessageBoxOnError -DHANGINGJAVA4260 -XX:CompileThreshold=2 -XX:+UseConcMarkSweepGC -XX:+CMSPermGenSweepingEnabled -XX:+CMSClassUnloadingEnabled -XX:-PrintVMOptions runtime.threads.ThreadInterruptTest3.ThreadInterruptTest3
[2007-02-15T23:50:40.73] ***++****++****+***+**
[2007-02-15T23:54:03.12] # Test level exit status: 0
[2007-02-15T23:54:03.23] ***++****++****+***+**
[2007-02-15T23:54:03.53] ***++****++****+***+**
[2007-02-15T23:54:03.94]

Comments

Note that as of JDK-6313903 Windows no longer uses the interrupt_event for sleeps, but instead uses the _sleepEvent ParkEvent. This removes the possibility of the races being addressed by this, and related, issues.
15-10-2019
Code review email from Karen Kinnear: 2/19/14 The code fix looks good. The question is whether to leave in the intrinsic or not since it could cause inconsistent data to be returned. One way to deal with this would be to modify the intrinsic to only optimize the case in which the thread is not interrupted, i.e. t = Thread.current() && (!TLS._osthread._interrupted) i.e. the intrinsic code appears to have two fast paths: first !TLS_Interrupted -> return false second: TLS._interrupted && !clear_int So I was proposing keeping the first fast path and getting rid of the second. 2/19/14 email from David Holmes: Right - I get it now. In simple terms the fast-path can only ever return false, and that can be determined just by reading the low-level _interrupt field. To return true you must read the _interrupt field and check the the event state ie take the slow path. That ensures we can never see: fastpath -> true slowpath -> false I still wonder though whether this intrinsic has any real value. It optimizes the Java code for: Thread.currentThread().isInterrupted(); which is not used much at all. Thanks. David
25-02-2014
By checking: WaitForSingleObject(osthread->interrupt_event(), 0) == WAIT_OBJECT_0 we can get if the Event gets signaled. Fix: --- a/src/os/windows/vm/os_windows.cpp Thu Jan 09 15:48:00 2014 -0800 +++ b/src/os/windows/vm/os_windows.cpp Wed Jan 15 16:04:24 2014 -0800 @@ -3613,7 +3613,7 @@ "possibility of dangling Thread pointer"); OSThread* osthread = thread->osthread(); - bool interrupted = osthread->interrupted(); + bool interrupted = osthread->interrupted() && (WaitForSingleObject(osthread->interrupt_event(), 0) == WAIT_OBJECT_0); we can get away with interrupted field but still need to reset Event so i keep as it is to be consistent with other platforms.
16-01-2014
Note that this problem also exists with Object.wait(), but in that case the spurious return is allowed for by the spec - however it is preferable not to have spurious wakeups even if allowed.
13-01-2014
The check for interruption on entry to the sleep is desirable and works fine on every platform except Windows. There is nothing wrong with doing the early check, the problem is in the Windows implementation of interrupt support using the interrupt event. So I do not support changing common code that works as expected and desired on all platforms but Windows. Using a distinct Event to represent interruption is appealing on Windows because we can use WaitForMultipleObjects to easily detect normal notification versus interruption, and it also exposes timeouts very cleanly. In contrast the other platforms, using condition variables, have to check secondary state to see if they were interrupted or notified. However the problem with using the distinct Event is that we now have two things that represent being interrupted: the field, and the Event; and we can not manage these two atomically. There may be another way out here. We may be able to do away with the interrupted field and simply use the Event to answer the query is_interrupted. If you call WaitForSingleObject on the Event, with a timeout of zero, it will return immediately if the Event is not signalled (or if it is of course) and you can then use the return code to determine the reason: bool os::is_interrupted(OSThread osthread) return WaitForSingleObject(osthread->interrupt_event(), 0) == WAIT_OBJECT_0; http://msdn.microsoft.com/en-us/library/windows/desktop/ms687032%28v=vs.85%29.aspx Atomicity is restored because there is now only one entity that represents the interrupt state. This is somewhat heavyweight compared to using the field so we should run some basic benchmarks.
12-01-2014
David, I just described what I got in more detail, yes, it basically a reiteration of what you logged. The problem caused by: JVM_Sleep: JVM_ENTRY(void, JVM_Sleep(JNIEnv* env, jclass threadClass, jlong millis)) JVMWrapper("JVM_Sleep"); if (millis < 0) { THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "timeout value is negative"); } if (Thread::is_interrupted (THREAD, true) && !HAS_PENDING_EXCEPTION) { THROW_MSG(vmSymbols::java_lang_InterruptedException(), "sleep interrupted"); } We check if the thread is interruped, if yes, return early without waiting on Event. I think this code is not necessary and should be removed. Upon on return from real interrupt, the Event in fact is the sync object so it will not wake up spuriously. Testing with this removed shows no regression.
10-01-2014
Yumin, you have just re-stated what I already reported back in 2011.
10-01-2014
SetEvent and setting interrupted should be atomic. The failure comes like: job2 is interrupting job1 which is in JVM_Sleep, but not sleeping on Event, just before it reads is_interrupted, now since it is set by job2 to true, it is 'interrupted' and throws IE (InterruptedException), in is_interrupted(Thread* thread, bool clear_interrupt), since we use 'true' for second parameter, it reset Event, and set interrupted to 'false' again. Now, if the job2 has no chance to go forward, and still hold by OS, job1 still goes on and print out, interrupt job2, then go back to sleep again. Note, now since it is 'uninterrupted' and Event reset, so it will land on WaitForMultiObjects and sleep. Now, job 2 has chance to go, and do 'SetEvent' which release job1 from waiting. If now OS switches job1 back and executes it, it will throw IE again, so we see consecutive output from job1. What we need is atomic setting for both Event and interrupted.
09-01-2014
Are you sure both the threads have been started before any interrupt is issued? What you seem to be seeing here is a different problem to the original.
06-01-2014
To easily indicate the failure patter, I modified the output to job, means, 121212..., also added 'Y' after job1 and 'X' after job2. According to the java code, if failed, will be 1122... This can be reproduced on Solaris (below), I haven't tried on Linux. public void job1() { for (int i = 0; i < 2000; i++) { try { sleep(5000); System.out.print("Y"); } catch (InterruptedException e) { System.out.print(job); other.interrupt(); } } } public void job2() { for (int i = 0; i < 2000; i++) { other.interrupt(); try { sleep(5000); System.out.print("X") } catch (InterruptedException e) { System.out.print(job); } } } YX112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221122112211221121221122112212112211221122112211221122112211221122112211221122112211221122112212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121212121 'Y' means job1 sleep time out. The fist interrupt from job2 not caught. Thread2(job2) goes into sleep, timeout, then print 'X', then goes to second round iteration, send second interrupt to job1. At the interrupt time, job1 is in sleeping. The first interrupt operation from job2 has no effect else we can cannot see 'Y' print first. That could be thread2 run first, interrupt action happens first before job1 sleep, look like job1 sleep wakes spuriously and goes into next iteration without catching IE. I did see the pause for 5 seconds (the sleep time) before this output came out.
27-12-2013
Following script, running on my laptop Win7. with 6u45 failed: #!/bin/bash $JAVA_HOME/bin/java -server -Xbatch -XX:-ShowMessageBoxOnError -DHANGINGJAVA4260 -XX:CompileThreshold=2 -XX:+UseConcMarkSweepGC -XX:+CMSClassUnloadingEnabled -XX:-PrintVMOptions runtime.threads.ThreadInterruptTest3.ThreadInterruptTest3 > stdtest.out 2>&1 iter=0 while [ $iter -lt 100 ] do echo "Iteration $iter" $JAVA_HOME/bin/java -server -Xbatch -XX:-ShowMessageBoxOnError -DHANGINGJAVA4260 -XX:CompileThreshold=2 -XX:+UseConcMarkSweepGC -XX:+CMSClassUnloadingEnabled -XX:-PrintVMOptions runtime.threads.ThreadInterruptTest3.ThreadInterruptTest3 > test.out 2>&1 diff test.out stdtest.out if [ $? -ne 0 ]; then echo "Failed" exit 0 fi let iter+=1 done
26-12-2013
Reproduced with 8 most recent version.
26-12-2013
Yumin: try using a far less capable machine (only a couple of cores) and bump up the number of runs to a few hundred at least.
18-12-2013
No such a check would not suffice as we could still race with a second interrupt(). It would probably fix the current observed test failure (as only one thread interrupts another).
18-12-2013
The fundamental problem here is that the manipulation of the interrupt state and the manipulation of the event is not atomic. No matter which way round we do things it will leads to different races. Now the manipulation of the ParkEvent and Parker are also not done atomically wrt. the interrupt state, but those constructs are already defined to allow spurious wakeups and so the usage of those constructs deals with them. Somehow we need to deal with this "spurious wakeup" for the interrupt_event. That implies we may need to re-wait and if we are going down that path then we should address the early-return-from-sleep problem. As only the current thread can clear its interrupt state, it may suffice to check for being interrupted, to determine that this is a "spurious wakeup".
18-12-2013
I think there is a second race here that can lead to a "spurious wakeup" from the interrupt_event that we sleep on. os::interrupt does: 1. osthread->set_interrupted(true); 2. OrderAccess::release(); 3. SetEvent(osthread->interrupt_event()); os::is_interrupted does: 1. bool interrupted = osthread->interrupted(); if (interrupted && clear_interrupted) { 2. osthread->set_interrupted(false); 3. ResetEvent(osthread->interrupt_event()); } Suppose that we call ResetEvent _before_ SetEvent. The os::sleep will see we were interrupted and throw IE. But the next time we do a sleep in that same thread and the interrupt is not set, the WaitForSingleObject will return immediately due to the earlier SetEvent. os::sleep assumes a non-timeout is an interrupt and so throws IE again, so the test thinks everything is going normally and reaches the second print statement without an intervening interrupt actually occurring.
18-12-2013
This may have been a premature closure. While the failure is now rare I can still get this test to fail occasionally. The failure mode is as follows: - thread 1 reports double use of '+' and then throws an error indicating that the interrupt got posted during the print operation - thread 2 then times out of its sleep and throws an Error to indicate the sleep returned I think this test has been modified since this was reported as I don't see how the 6535709 bug could lead to this behaviour. That bug would case Thread.sleep to not get interrupted which means we would throw an Error as the sleep returned normally.
18-12-2013
So far the indications are that this is a bug in the win32 Event system. The threads are not being released from WaitForMultipleObjects even though they are being interrupted, which should set the interrupt_event and end the wait. I tried "tricking" the system by adding dummy events and by switching from manual-reset to auto-reset events, but it had no effect. There are no errors being reported by any of the event functions.
18-12-2013
I've been able to reproduce this on tulipwood.east. It seems to be the same problem as 6535709 - the interrupt is getting lost. So both threads end up in sleep() at the same time and stay there until the sleep expires. This causes incorrect output.
18-12-2013
Changed the synopsis to describe the problem as this has nothing to do with join() This test is "failing" very often in the nightly tests - mainly on Windows but once even on sparc! The odd thing is that for many of those "failures" I can not find any error in the output - it appears to be an alternating sequence of + as expected. It would be good to eliminate all possible errors related to performing the I/O, writing the file and performing the diff. To that end the test could be trivially rewritten to be self-testing: static char last = ''; // whichever char is expected second static sychronized void print(char ch) { if (ch == last) System.exit(-1); last = ch; }
18-12-2013
The most likely problem is that a sleep is returning very early. This could be verified by adding a "throw new Error("sleep returned normally");" after the sleep. We can then check how much time is actually elapsing. If that is the cause and we can reproduce this fairly easily then we may be able to determine the underlying cause of the sleep returning early.
18-12-2013
The synopsis and description of this bug is incorrect. There is only a single join() per-thread in the test. The problem is that we have two threads working in lock-step, sleeping and interrupting the other and printing either * or +. Because they operate in lock-step the expectation is that the output is always a sequence of alternating * and + Examining the output shows that this is not the case. Somehow operations are occurring out of the expected order.
18-12-2013
I tried to reproduce but fail to do so. Used jdk1.8.0, jdk1.7.0_60 (the most recent) and jdk1.7.0_u40, jdk1.6.0_01, jdk1.6.0_07 and recent jdk1.6.0_60. The output all looks right(every version run over 20 times): $ /cygdrive/j/6u1/promoted/latest/binaries/windows-amd64/bin/java -server -showversion -Xbatch -XX:-ShowMessageBoxOnError -DHANGINGJAVA4260 -XX:CompileThreshold=2 -XX:+UseConcMarkSweepGC -XX:+CMSClassUnloadingEnabled -XX:-PrintVMOptions runtime.threads.ThreadInterruptTest3.ThreadInterruptTest3 java version "1.6.0_01" Java(TM) SE Runtime Environment (build 1.6.0_01-b06) Java HotSpot(TM) 64-Bit Server VM (build 1.6.0_01-b06, mixed mode) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ I am using a Dell T7600 32CPUs, 32GB Mem, Windows 7.
16-12-2013
Moving to JDK 9, too risky to make any changes for 8.
27-08-2013
EVALUATION There is an additional race between os::interrupt and os::is_interrupted that can result in the interrupt state being cleared, but the interrupt_event remaining "set". This will cause the next call to os::sleep to return immediately, as-if it were interrupted, and so throw InterruptedException.
21-03-2011
SUGGESTED FIX bool os::is_interrupted(Thread* thread, bool clear_interrupted) { assert(!thread->is_Java_thread() \|\| Thread::current() == thread \|\| Threads_lock->owned_by_self(), "possibility of dangling Thread pointer"); OSThread* osthread = thread->osthread(); bool interrupted; interrupted = osthread->interrupted(); ! if (interrupted && clear_interrupted == true) { osthread->set_interrupted(false); ResetEvent(osthread->interrupt_event()); } // Otherwise leave the interrupted state alone return interrupted; }
20-03-2011
EVALUATION There is a bug in the win32 version of os::is_interrupted. There is always a race between an interrupt that will call SetEvent and the WaitForSingleObject call that blocks the thread. By first checking the interrupt state the sleep code expects to see if an interrupt has occurred, and if not it will call WaitForSingleObject, knowing that an interrupt after the check will call SetEvent and release the thread. The bug in the Windows version of os::is_interrupted(true) means that it can return false (causing the thread to call WaitForSingleObject) but also clear the interrupt state and call ResetEvent prior to returning false. As a result the fact the interrupt occurred has been lost and so the sleep will last for the full time value specified. The win32 version of os::is_interrupted differs from the Solaris and Linux versions which do not have this bug. bool os::is_interrupted(Thread* thread, bool clear_interrupted) { assert(!thread->is_Java_thread() \|\| Thread::current() == thread \|\| Threads_lock->owned_by_self(), "possibility of dangling Thread pointer"); OSThread* osthread = thread->osthread(); bool interrupted; interrupted = osthread->interrupted(); if (clear_interrupted == true) { osthread->set_interrupted(false); ResetEvent(osthread->interrupt_event()); } // Otherwise leave the interrupted state alone return interrupted; } Here we return the original value of interrupted, but that value could have changed between the read and the subsequent clear - in which case the intermediate interrupt is lost. In contrast on Solaris we have: bool os::is_interrupted(Thread* thread, bool clear_interrupted) { assert(Thread::current() == thread \|\| Threads_lock->owned_by_self(), "possibility of dangling Thread pointer"); OSThread* osthread = thread->osthread(); bool res = osthread->interrupted(); // NOTE that since there is no "lock" around these two operations, // there is the possibility that the interrupted flag will be // "false" but that the interrupt event will be set. This is // intentional. The effect of this is that Object.wait() will appear // to have a spurious wakeup, which is not harmful, and the // possibility is so rare that it is not worth the added complexity // to add yet another lock. It has also been recommended not to put // the interrupted flag into the os::Solaris::Event structure, // because it hides the issue. if (res && clear_interrupted) { osthread->set_interrupted(false); } return res; } Note that we only clear the interrupt if we saw that we were interrupted. In this way we guarantee that if we return false we did not modify the interrupt state - or for the Windows case the interrupt event state - hence an interrupt can not be lost. The Note in the code above indicates there was some awareness of race conditions in this area, but for some reason this did not get carried over to the Windows code. It also does not address this specific condition, and unlike the spurious wakeup situation the current behaviour with Thread.sleep is definitely a bug.
19-03-2011