How to fix this concurrent access to a variable

#What I am trying to achieve : I want a loop that updates the "i" variable in the "Subprocess" class. I want to print out the variable at each iteration and kill the thread when the variable reaches the value of 50.

#What I am getting : an infinite loop printing out 100

The chance that the main thread will ever observe currentValue to be exactly equal to 50 is vanishingly small. There are a few reasons for this.

First, threads have the opportunity to perform many operations before they are unscheduled. A processor can increment a counter 50 times in absolutely no time at all. The Subprocess will likely reach 100 before the other thread has even had a chance to check the current value more than once.

Secondly, even if the Main thread had a chance to call thread.getI() many times per counter increment, what it reads is likely not the same value that Subprocess wrote in the updateI() method. That's because you didn't synchronize the two threads.

There are many ways to "fix" your application, but it's hard to discuss their advantages and disadvantages, because your application doesn't do anything useful.

Stephan van Hulst wrote:[code=java]

There are many ways to "fix" your application, but it's hard to discuss their advantages and disadvantages, because your application doesn't do anything useful.

Hi! Thank you for your answer and for clearing that up. I know it does not anything useful, but it's for didactic purpose and I'd like to learn how I could avoid running in these issues in the future

Well, it really depends on how you expect the two threads to interact.

Do you want the Subprocess to increment the counter, and then stop incrementing until the Main thread has had a chance to check the value?

Do you want the Subprocess to run slower, so that the Main thread has a chance to check the value more often?

Stephan van Hulst wrote:

Do you want the Subprocess to increment the counter, and then stop incrementing until the Main thread has had a chance to check the value?

Do you want the Subprocess to run slower, so that the Main thread has a chance to check the value more often?

Well, theoretically the second solution sounds better to me. But if possible I'd take a glance at both to widen my knowledge a bit. Thank you in advance

Before I show you how to properly synchronize the threads, I will first rewrite your original program to fix the following issues:

Only make classes and methods public when you really need to.

Make classes final unless you have a really good reason not not.

Don't declare variables until you need them. The currentValue variable could have been declared inside of the while-loop.

Don't extend Thread to create runnable tasks. Implement Runnable instead.

Don't use Thread to execute tasks. Use an ExecutorService instead.

Use i++ instead of i = i + 1.

The only change I made that actually impacts the result of execution is that I changed the check condition to count >= 50. This way, the application will at least terminate at some point. Here is an example of some output:

As you can see, before the Counter starts running, the Main task has time to check task.getCount() many times. Then, when the Counter finally starts running, it almost immediately hits 170 before the Main task checks it again.

Now, one approach you could take to improve the application is to make two small changes:

Slow down the two threads by making them sleep for a small amount of time between updates.

Make the count field volatile, so that when the Main thread reads it, it will really actually read the last value that Counter updated it to, and not some cached value.

Inside Main.java:

Inside Counter.java:

And another example output:

Output looks a lot better: The application actually stops when the counter reaches 50, and there are no more gaps between different increments.

It comes with a few downsides though. First, the Main task still pointlessly polls for updates even though the Counter hasn't updated anything. During this time, it wastes CPU cycles. Secondly, Thread.sleep() is unreliable and you should not depend on it.

Instead, in real applications you usually carefully synchronize two threads by using locks and conditions:


This yields the following output:

Stephan van Hulst wrote:

I can't thank you enough for your answer, all the suggestions and for all the "cues" in your code I will look further more in. Thank you so much!

Stephan van Hulst wrote:Use i++ instead of i = i + 1.

Were you referring to conciseness or presumable atomic operation?

Just wanted to mention, I think ++ operator (pre and post)  isn't atomic. It may not matter in the particular example above, but in some variants it may. In such cases worth being aware of  AtomicInteger.

Liutauras Vilda wrote:. . . ++ operator . . . isn't atomic. . . . .

I didn't find anything in the JLS, but Baeldung says it comprises three separate operations.

Addition: I tried it on JShell with two Threads and got the wrong result, so ++ definitely isn't atomic.

My suggestion was mostly because i++ is idiomatic. Conciseness is a small bonus too.

i++ is indeed not atomic, but that doesn't matter because atomicity is guaranteed by the Lock.