• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

synchronized doesn't work as expected

 
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi,
Here is the simple synchronization testing code:



I expect to see output 500, but it's not the case.
Could you please help understand what am i doing wrong here?
 
author
Posts: 23951
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sergey Izg wrote:
I expect to see output 500, it's not the case.
Could you please help understand what am i doing wrong here?



First, welcome to the ranch.

You have two issues. One. Your getInstance() method is not thread safe. It is possible for different threads to have different data instances. Two. Your updateUnit() method is not thread safe. The operation is not atomic, and it is this issue that is causing your result to be less than expected.

Henry
 
Bartender
Posts: 1464
32
Netbeans IDE C++ Java Windows
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Henry Wong wrote:

Sergey Izg wrote:
I expect to see output 500, it's not the case.
Could you please help understand what am i doing wrong here?



First, welcome to the ranch.

You have two issues. One. Your getInstance() method is not thread safe. It is possible for different threads to have different data instances. Two. Your updateUnit() method is not thread safe. The operation is not atomic, and it is this issue that is causing your result to be less than expected.

Henry



I think Henry meant this, but I'll emphasize that both methods are contributing to your problem. To help see what's wrong in your factory method, add this at Line 3, below:

You are probably going to be suprised to see that it is creating more than one instance. Each new instance is being stored in the same place, so only the last one survives to the end of your program. Any updates to the others will be lost.

Remember that each thread runs as though it were simultaneous with the others. Thus, each thread can be testing to see if instance is null at the same time. When I run your code, all five threads saw instance as null, so all five created a new instance of data.

By the way, class names should always start with a capital letter.
 
Sergey Izg
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thank you very much, guys. i made data instance be initiated before start running the threads and it did a trick
 
Bartender
Posts: 2236
63
IntelliJ IDE Firefox Browser Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Those issues are called race conditions.
First one is check-then-act.

In general the task is:
1. Check a condition.
2. Act on a condition (decide what to do based on the condition).

In you case it is.
1. Check if a variable is null.
2. If it is null assign new object reference to it.

It might go like this (T1 and T2 are thread names):
T1 checks if a variable is null.
T1 sees it as null so it assigns new object reference to it.
T2 checks if a variable is null.
T2 sees is as not null so it does nothing.

Which is an execution you expect.

But it might also execute like this:
T1 checks if a variable is null.
T2 checks if a variable is null.
T1 sees it as null so it assigns new object reference to it.
T2 sees it as null so it assigns new object reference to it.

And you can observe two objects being created.

The second type of race condition is read-modify-write.

In general it goes as:
1. Read a value.
2. Modify the value.
3. Write the value.

In your case it is.
1. Read the counter from the variable.
2. Add one to the counter.
3. Write the counter to the variable.

It might go like this:
T1 reads value 15 from the counter.
T1 adds one to the value.
T1 writes 16 to the counter.
T2 reads value 16 from the counter.
T2 adds one to the value.
T2 writes 17 to the counter.

Which is an execution you expect.

But it might also execute like this:
T1 reads value 15 from the counter.
T2 reads value 15 from the counter.
T1 adds one to the value.
T1 writes 16 to the counter.
T2 adds one to the value.
T2 writes 16 to the counter.

And you can observe one increment being lost.
 
Sergey Izg
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Paweł,

But it might also execute like this:
T1 reads value 15 from the counter.
T2 reads value 15 from the counter.
T1 adds one to the value.
T1 writes 16 to the counter.
T2 adds one to the value.
T2 writes 16 to the counter.


But shouldnt it be synchronized? Should T2 not have access to the method while T1 handles it? Or, synchronized is not about read?

Thanks
 
Paweł Baczyński
Bartender
Posts: 2236
63
IntelliJ IDE Firefox Browser Spring Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You are right. This is another way it could execute. There are others.

Synchronized on a method guarantees an exclusive access only to one method at a time.
So, read and write are atomic as they are synchronized.

But this does not help you much.
T1 can read the value in atomic way. Once it is done T2 can read the value in atomic way before T1 updates the value.

In pseudocode you currently have: ...which is not atomic. Something might happen in other thread between those synchronized blocks.

You need the entire read-modify-write sequence to be atomic.

One way of ensuring that is to provide one synchronized method that does the job.
In pseudocode it would be:

Other way is to use a specialized thread-safe class like AtomicInteger or LongAdder.
 
Sergey Izg
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
So, will the below be more right?

 
Paweł Baczyński
Bartender
Posts: 2236
63
IntelliJ IDE Firefox Browser Spring Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
That will do the job.

Please, don't use [strike] tags inside [code] tags.
I have changed them to block comments this time.
 
Sergey Izg
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks
 
Stevens Miller
Bartender
Posts: 1464
32
Netbeans IDE C++ Java Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Very nice guide for a Greenhorn, Paweł. Have another cow for your herd.
 
Paweł Baczyński
Bartender
Posts: 2236
63
IntelliJ IDE Firefox Browser Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Also, you could have unit.updateUnit() synchronized and this would not require changes to data class.
 
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sergey Izg wrote:Or, synchronized is not about read?


I think Pawel's summed it up for you, but there are many things that simple synchronization can't guard against. It is an exclusion protocol, pure and simple, but it does NOT guarantee the order in which excluded Threads execute.

It also only works on whatever action is done in the method, so it can't, for example, lock out an entire dataset while you sort it, unless you also synchronize the sort() method.

IMO, it's almost too simple because, while it guarantees method atomicity, it doesn't guarantee functional atomicity. That's your job ... and unfortunately, it's easy to get wrong.

You might also want to read about ReentrantLocks (←click), because they provide quite a bit more control (and possibly speed) - albeit at the expense of simplicity.

Good luck.

Winston
 
This guy is skipping without a rope. At least, that's what this tiny ad said:
a bit of art, as a gift, the permaculture playing cards
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic