On Mon, 22 Mar 2021 05:40:10 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> test/jdk/java/util/Timer/AutoStop.java line 47: >> >>> 45: public void run() { >>> 46: tdThread = Thread.currentThread(); >>> 47: synchronized(wakeup) { >> >> tdThread should be set inside the sync block, and then doesn't need to be >> declared volatile > > Without volatile is the while loop still okay? And the read for the join > call? Yes the while loop is okay because the access is within a synchronized block. When the loop exits tdThread is seen as non-null by the current thread, and that field is never written to again, so when the current thread does the join() it has to be using the non-null value of tdThread that it previously saw. >> test/jdk/java/util/Timer/AutoStop.java line 67: >> >>> 65: t.schedule(new TimerTask() { >>> 66: public void run() { >>> 67: ++counter; >> >> This is not thread-safe. Operations on volatile variables are not atomic. > > Only one thread (the timer's thread) is writing, via a sequential series of > task executions, so the simple increments are fine. I made `counter` > volatile because it's being written by one thread and read by a different > thread. Happy to drop the qualifier if that's not needed. Apologies - I thought there was real concurrency going on there. :) The volatile is correct for the reason you cited. ------------- PR: https://git.openjdk.java.net/jdk/pull/3106