• 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

How to use notifyAll() to resume a thread in Java

 
Greenhorn
Posts: 11
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm trying to use Object.notifyAll() method to resume a thread that is pasued with Object.wait() in Java. Currently, a client will connect to a socket, then a condition is asked, if the client hasBall is true or not. If it's true, the client can pass the ball via a method to another client. If not, the client must wait to receive the ball - this is where lock.wait() is called.

The problem is that the lock.notifyAll() call is not waking the lock.wait() call.

Code:



Expected Outcome

The first client joins with the ball. A second client joins without a ball. Client 2 is waiting for the ball. Client 1 passes the ball to Client 2. Client 1 updates, no longer has ball. Client 2 updates, now has the ball.

Actual Outcome

As expected, BUT Client 2 will not update when it receives the ball.

(note: I also posted this question on stackoverflow incase I get an answer there: https://stackoverflow.com/questions/70047600/how-to-use-notifyall-to-resume-a-thread-in-java)
 
Rancher
Posts: 326
14
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
1) Please update your SO post to include a link back here so SO users know it's cross-posted.
2) Although I'm not good with concurrency myself maybe the java tutorials help you along: https://docs.oracle.com/javase/tutorial/essential/concurrency/index.html
 
Marshal
Posts: 79177
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Adding to our threads forum.

If I remember correctly, the notifyXXX() methods only have a real effect on a thread which is not executing becasue wait() has previously been called. Use notify() if you don't mind which of the threads is woken, because they are all doing the same thing. Otherwise notifyAll(), but there is no guarantee which thread will receive the notification.
 
Bartender
Posts: 5465
212
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
In main in ClientInterface you create one CI and one Client. In your description, you talk about Client 1, Client 2, etcetera.

Can you show us the full code, or at least the Client class?
 
Sean Mcleod
Greenhorn
Posts: 11
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator


In main in ClientInterface you create one CI and one Client. In your description, you talk about Client 1, Client 2, etcetera.

Can you show us the full code, or at least the Client class?



I've updated my post with the Client class. When I say "Client 1, Client 2" etc, I mean when I start another instance of the ClientInterface.java in my IDE
 
Sheriff
Posts: 5555
326
IntelliJ IDE Python Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Perhaps it would be better if you added to your code in a new post. Sometimes when you edit existing posts it makes the replies sound confusing, for example in this case "Why is Piet asking for the code for the Client class when it's right there?".

Think of a coderanch thread as a conversation where time moves forwards only and resist the temptation to go back in time a rewrite history. In other words, provide your Client class code in a reply.
 
Sean Mcleod
Greenhorn
Posts: 11
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Client class:

 
Sean Mcleod
Greenhorn
Posts: 11
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
So I think I can answer why this isn't working now. I didn't realise that each process in Java belongs to its own JVM instance and therefore cannot communicate to threads in another process.
 
Rancher
Posts: 5008
38
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

cannot communicate to threads in another process.


Which parts of your project were executing in separate JVMs?  
A way for code in separate JVMs to communicate is via Sockets.
 
Saloon Keeper
Posts: 15510
363
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Is this an exercise? If not, then avoid using Object.wait() and Object.notify()/Object.notifyAll(), and instead use ReentrantLock.newCondition(), Condition.await() and Condition.signal()/Condition.signallAll().

There are other issues with your code:
  • Never extend Thread. Implement Runnable or Callable tasks instead and pass your tasks to an ExecutorService.
  • Make your classes final.
  • Make your fields private. This is especially important for locks.
  • Never use static fields for anything other than constant values.
  • Use descriptive method signatures. There is absolutely no indication that pass(playerId) prompts the user for an ID of the player you want to pass the ball to, instead of using the ID that you passed to it.
  • Think about what exceptions are appropriate for a method to throw. Declaring that a method throws Exception is rarely appropriate.
  • Don't print to System.out directly. Encapsulate user interaction in a separate class.
  • If your class encapsulates I/O, implement Closeable, not AutoCloseable.
  • Never create resources in constructors. Sockets and streams may be passed to a constructor, but not created or used therein.
  • Methods that read input should not start with get-, but rather with read-. Getters must be simple and return rapidly.
  • Don't use compareToIgnoreCase() when equalsIgnoreCase() will do.

  • I'll see if I can post a good example that uses ReentrantLock later.
     
    Piet Souris
    Bartender
    Posts: 5465
    212
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    I ran your code, albeit with only reader = new Scanner(System.in), since that port gave me an error.

    I do not understand the code. First of all, the client is constantly asked for a list of players, then it asks for the player with the ball.

    In ClientInterface, I have player 3 with the ball and myself is player 2. Then this code is entered:

    How do we get out of this part? Should't pass() be called so there is a chance that myself #2 gets the ball sometime? And why must all the client data be typed in over and over again? But then again: I might miss something.

    Like Stephan, I wrote a piece of code that uses a ReentrantLock, and whoever gets the lock has the ball, so to speak, and the first to get 10 balls wins. But I realized it had nothing to do with your game, so never mind.
     
    Sean Mcleod
    Greenhorn
    Posts: 11
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    It's a client server system. I managed to kind of fix the issue. So initially (ignorantly..) I tried to make clients who did not have the ball, wait to receive a ball. But this doesn't work since waking up a waiting thread with "notifyAll" does not work since I'm running multiple processes (aka running multiple instance of ClientInterface.java). I instead create a Thread for each client that connects to the server for a class "ClientHandler".



    It's an assignment based on using a client server system using a text based protocol
     
    Sean Mcleod
    Greenhorn
    Posts: 11
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Updated ClientInterface:

     
    Campbell Ritchie
    Marshal
    Posts: 79177
    377
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Anything with empty catches in is presumed to be incorrect until proven otherwise.
    Why are you using an Object as a lock, rather than a Lock instance? Why do you have wait() calls not inside a loop?
     
    Sean Mcleod
    Greenhorn
    Posts: 11
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    Why are you using an Object as a lock, rather than a Lock instance?



    Would it change the functionality? The object is working as a lock so I didn't see the need to use the locks that inherit from the Lock interface, but could be wrong.


    Why do you have wait() calls not inside a loop?


    I need the client interface process to update (aka call the printInfo() method again) when:
    - a new player joins
    - the player with the ball has passed it to another player

    This was the only way I could get it to to work. To update the client interface when a new player joins is confusing, I only want to update the client processes that exist before a new player joins (so they are aware a new player has joined), but currently the new player will also be updated on joining (well, updated on the fact that they have joined...so doesn't quite make sense).
     
    Campbell Ritchie
    Marshal
    Posts: 79177
    377
    • Likes 1
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    Sean Mcleod wrote:. . . Would it change the functionality? . . .

    Probably not in  a simple case like this. What would happen if you change line 64 to the following?
     
    Sean Mcleod
    Greenhorn
    Posts: 11
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    What would happen if you change line 64 to the following?


    It fixed the duplicate printing for new clients, thanks! So am I right in saying this works now because, instead of all threads acquiring the intrinsic lock on the previous object "lock" (because it is a static obejct lock), by changing the lock to the class itself, only the thread which the client process is connected to, will acquire the lock? Or am I confused?
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 15510
    363
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    The difference between Object and Lock is mostly a matter of idiom. Technically they work very similarly, it's just that Lock expresses your intention a lot better, which makes the code easier to read and maintain.

    Lock has another advantage though. It allows you to create Conditions that you can await(), which is great because it allows you to signal only those threads that are waiting on a specific condition, and not all threads that are waiting. It also increases the readability of your code further.

    What Campbell meant when he asked you why you don't have the wait() calls inside a loop, is that you must ALWAYS put a wait() call inside a loop that checks the condition. That's because wait() might randomly stop waiting, even if you didn't notify the thread explicitly.

    Here's what the entire idiom looks like when you use Lock instead of Object:

     
    Stephan van Hulst
    Saloon Keeper
    Posts: 15510
    363
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    The code above also demonstrates how to use multiple conditions. Even though receivedBallCondition and lostBallCondition are both based on the same Lock, only those threads that are waiting on receivedBallCondition will wake up when you call giveBall(). Threads that are waiting on lostBallCondition will only wake up if you call takeBall(). This is an improvement over Object.wait(), which will always wake up regardless of where in your code you call Object.notifyAll().
     
    Piet Souris
    Bartender
    Posts: 5465
    212
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    I said that my code had nothing to do with OPs game, it does not use Sockets or ports. But here goes anyway, since I tried some things that I normally never use, so for me it was a good practise.

    I use a Lock too, as Stephan did, but I did not use Conditions (never used so far). The game nas a number od Players (each Player being a Runnable) and each is launched in a Threadpool. At the start, one has the ball. It then locks on the Lock, increases its score, and if the score has reached the maximum, that player is removed from the playerlist and added to the resultlist. It then passes the ball to a random other player (or keeps the ball). Etcetera, until all players have finished. If a player does not have the ball, it goes to sleep for 100 ms before checking the status.

    But I'm not sure about some things: is a Lock necessary? How about the visibility of all players and their field "hasBall"? The code seems to work, but if anyone has some useful comments, I'd like to hear them.

     
    Stephan van Hulst
    Saloon Keeper
    Posts: 15510
    363
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    Piet Souris wrote:I use a Lock too, as Stephan did, but I did not use Conditions (never used so far).


    You'd use Condition in the same situation where in the olden days you would use Object.wait(). Lock is a replacement for the synchronized keyword, not for wait().

    But I'm not sure about some things: is a Lock necessary?


    Yes. Each player is performing non-atomic operations on variables that are shared with other players. Without the lock, two types of problems may arise:

    First, other players may observe that individual operations occur out-of-order. For instance, if player 1 receives the ball and has reached the maximum score, it will first add itself to the list of "ranked" players before it passes the ball to another player. However, the player that you passed the ball to may not observe the change to the result list, and if it also reaches the maximum score, it might "steal" the rank of player 1, or even worse, completely overwrite player 1's operation so that player 1 doesn't appear in the final ranking at all. Worse yet, its possible that player 2 removes itself from the list of players after player 1 has checked players.isEmpty(), causing player 1's thread to crash with an IndexOutOfBoundsException when it calls players.get(r).

    Secondly, updates may not be observed by other threads at all. When player 1 calls getBall() on player 2, player 2 may never actually observe the change to hasBall if you don't synchronize the two threads using Lock.lock(). In fact, your current code may theoretically display this problem because the hasBall check before the sleep() call is not synchronized.

    You didn't put the hasBall check inside the lock, because it would cause your application to deadlock with all players waiting for the active thread to stop sleeping, which will never happen because none of the blocked threads will ever give it a ball. This is exactly what Condition is for: It allows the active thread to release the lock while it waits for another thread to update the application state. However, in this particular case the hasBall check is an atomic operation, so instead of using Condition you can solve this issue simply by making the hasBall field volatile.

    How about the visibility of all players and their field "hasBall"?


    As I explained above, access to the result and players lists is protected by the Lock, so you're good there. You haven't protected the hasBall check, but you can fix this by making the field volatile.

    The code seems to work, but if anyone has some useful comments, I'd like to hear them.


  • Make your classes final and package private. None of your classes or members need to be public, with the exception of the main() and run() methods.
  • Seriously, don't start threads from constructors. All that a constructor should do is initialize the object's fields. Move the code that starts execution of your players to a separate method that you call after the constructor returns.
  • Always call ExecutorService.shutdown() from a finally block. Otherwise, if an unexpected exception occurs or if the main thread times out while waiting for termination, the thread pool might keep running and your application won't terminate.
  • Your ternary expression can be simplified to just i == r.
  • Make hasBall volatile.
  • Use a better name than getBall(). It is extremely confusing because it looks like a simple getter, but it's actually a mutator. Use a name such as setHasBall() or receiveBall() or something.
  • If you don't expect a particular checked exception to be thrown, enforce it with an assertion.
  • Your assumption that "// we have the ball!" is incorrect. You used an if-statement, so the player will only wait 100 ms once and then try to acquire the lock regardless of whether they have the ball.
  • You shouldn't use comments like that in your code anyway. If you feel the need to clarify what a piece of code does, lift it to its own method.
  • Call Lock.lock() outside of the try-statement. Even though lock() doesn't throw any exceptions, it's a good habit because similar methods like lockInterruptibly() do. If the attempt is made inside the try-statement, you will end up trying to unlock a lock that you haven't locked, which will result in an IllegalMonitorStateException.
  • Don't override equals() and hashCode() in classes that don't represent value types.


  •  
    Stephan van Hulst
    Saloon Keeper
    Posts: 15510
    363
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Oh, a final remark: It's better that you don't use awaitTermination(), because it may return before your tasks are done.

    If you want to wait until a bunch of tasks are done, use CompletableFuture.allOf():
     
    Don't get me started about those stupid light bulbs.
    reply
      Bookmark Topic Watch Topic
    • New Topic