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

Reply via email to