This week's giveaway is in the Cloud/Virtualization forum.
We're giving away four copies of Production-Ready Serverless (Operational Best Practices) and have Yan Cui on-line!
See this thread for details.
Win a copy of Production-Ready Serverless (Operational Best Practices) this week in the Cloud/Virtualization forum!
  • 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 all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Bear Bibeault
  • Jeanne Boyarsky
  • paul wheaton
Sheriffs:
  • Junilu Lacar
  • Paul Clapham
  • Knute Snortum
Saloon Keepers:
  • Stephan van Hulst
  • Ron McLeod
  • Tim Moores
  • salvin francis
  • Carey Brown
Bartenders:
  • Tim Holloway
  • Frits Walraven
  • Vijitha Kumara

Have trouble understanding Synchronization  RSS feed

 
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

I am brushing up on Thread Synchronization and I have trouble understanding something.

I have tried to solve a problem asking me to write a program that uses threads to add 5 user-input numbers.

Specifically, I have trouble with how setVal and getVal methods are set up in Val class in the code below.


The code works, but I do not understand why I need to use both wait and notify in both setVal and getVal.

In my previous attempts, I used only wait in getVal to make it wait until val is set by setVal method,

and I only used notify in setVal to alert the waiting thread in getVal of newly available val, thinking it will synchronize the threads and add the numbers up.

However, it did not work correctly and the program locked up with errors. I could not understand why it would not work and spew out errors.

Now, that I have it working, I still have problem seeing exactly why my previous attempts failed miserably.


Can someone explain to me why my program works now that I am using both wait and notify in both methods and why my previous approach did not work?


Thanks in adavance!


 
Saloon Keeper
Posts: 9986
206
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to CodeRanch!

Your first attempt would have been correct if your application only had to read and add 1 value. But that's not the case, you want to read and add 5 values. And every time you read a new value from the scanner, you must wait until the previous value has been added, right? Otherwise, it's possible that the setVal() methods sets a new value while the old value hasn't been read yet, overwriting it. Then, the input thread will have read 5 values, while the adder thread will only have had the chance to add 4 values, getting stuck forever waiting for a new value that never comes.

Now, I do find it odd that with the long delay it takes for the user to enter values, the adder thread wouldn't get a chance to perform the addition in between, but you can never be guaranteed of this. And I don't know the old version of your code, or I could have given you a better explanation.

Finally, there are a few issues with your code that I strongly recommend changing:

  • Write out names in full. Why is your synchronization primitive named Val, and not Value? Better yet, use a name that gives a clearer description of its purpose, which is making sure that set and get operations can only be performed in an interleaved fashion. Something like AlternatingAccessor.
  • Make classes final, unless you have a good reason not to.
  • Make fields private, unless you have a REALLY good reason not to. Make val private and name it value.
  • There's no point in adding members to a class with a higher visibility than that class, unless they're inherited. Give setVal() etc. default visibility.
  • Don't synchronize on this. Use a separate private lock object. The reason is that evil or buggy code can lock on your code, and then never release it, causing the rest of the application to deadlock.
  • Instead of using the synchronized keyword and the wait() and notify() methods, you can use the fancy new concurrency primitives in java.util.concurrency.
  • Don't use logical comparison operators on boolean literals. Instead of saying isNew == true, say isNew. Instead of saying isNew == false, say !isNew.
  • Always perform wait operations inside a loop. Instead of calling wait() inside an if-statement, call it inside a while-statement.
  • Don't just catch InterruptedException to ignore it. Most of the time, it's better to just let it propagate outside of the method, until there's a good place to handle it.
  • Use notifyAll() in preference of notify() in general. Your Val class doesn't know that there's only one reader and one adder thread at work. If it was shared by multiple readers and adders, it would fail.
  • Catch specific exception types. Most of the time catching Exception or even worse, Throwable, is a really bad idea.
  • Don't extend Thread. Instead, you can just override Runnable and pass that to a new instead of a Thread. There's an even better way though:
  • Don't use Thread at all. Instead, submit your Runnable tasks to an Executor.
  • Use compound assignment when possible. sum += val.GetVal() is more succinct than sum = sum + val.getVal().


  • Since you've already written code that satisfies the assignment, I'll show you how all of this looks when applied to the code you've written:



     
    Marshal
    Posts: 63781
    209
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Stephan van Hulst wrote:Welcome to CodeRanch!

    Welcome again

    . . .

  • There's no point in adding members to a class with a higher visibility than that class, unless they're inherited. Give setVal() etc. default visibility.
  • You can always override that method with higher visibility in a subclass.

    . . .

  • Don't just catch InterruptedException to ignore it. Most of the time, it's better to just let it propagate outside of the method, until there's a good place to handle it.
  • . . .

    Why is interrupted exception checked in the first place?
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 9986
    206
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    InterruptedException is one of those weird cases where a good argument can be made for either checked or unchecked. Personally, I'm a big fan of it being checked, because concurrent programming is already taxing enough on the mind without having to remember that a thread can be interrupted from anywhere at any time.

    It used to be a huge pain in code where you controlled all threads, and you knew for sure that it couldn't happen. You can write a helper method for that:



    In a lot of cases this is not necessary however, since a lot of utilities provide both interruptible and uninterruptible variants of a blocking method. For instance, Lock has lock() and lockInterruptibly(), and Condition has await() and awaitUninterruptibly().

    The most common case for wanting to ignore an interrupt is from Thread.sleep(), and instead of using Thread.sleep(), you will want to schedule a task that performs whatever it is that you want to delay.
     
    Campbell Ritchie
    Marshal
    Posts: 63781
    209
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Thank you
     
    jay shimazu
    Greenhorn
    Posts: 2
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I see it more clearly now. I suspected its because of multiple inputs and I guess I was very close!

    I really appreciate detailed feedback. It really helped me!

    Thank you
     
    Roses are red, violets are blue. Some poems rhyme and some don't. And some poems are a tiny ad.
    global solutions you can do at home or in your backyard
    https://www.kickstarter.com/projects/paulwheaton/better-world-boo
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!