• Post Reply Bookmark Topic Watch Topic
  • New Topic

Getting tasks to run in a separate thread

 
Joseph Macer
Ranch Hand
Posts: 63
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello, I have am working on improving the efficiency of my sending queue, a small setup so a separate thread can send objects over a network - so that other threads can add an object to be sent to that thread's queue, and go on doing whatever else while the sending thread processes the queue.

Currently, I have a system where if the queue is empty, I create a new thread and process the queue:


But I am almost always sending objects, so it makes little sense to spawn new threads and have them die off all the time. What I want to do is create a dedicated thread that sits idle when there is no work to do, and starts running again as needed, like this:



But things like that give me "IllegalMonitorStateException" on the wait line, which I don't understand - the javadoc says that means I do not own the object's monitor?

Any help would be greatly appreciated. Thanks!
[ August 01, 2008: Message edited by: Joseph Macer ]
 
Norm Radder
Bartender
Posts: 1526
14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
"IllegalMonitorStateException" on the wait line

Not sure where that happens in your code. Its best to post the full text of any error message!

wait() and notify() must be used in a synchronized block (ie own the object's monitor).
When a thread tries to enter a synchronzied block, the system checks if it is occupied by another thread and if not, allows the thread to enter and gives it the monitor. The next thread coming will put put in a queue to wait until.
[ August 01, 2008: Message edited by: Norm Radder ]
 
Joseph Macer
Ranch Hand
Posts: 63
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The code crashes on that "wait()" line in the sender class. I'm not sure what you mean by using it inside a synchronized block - I don't want it to wake itself up, I want the sender thread to go inactive when it has no work to do, and let ANY OTHER thread wake it up when there's more work to do.

Maybe wait() and notify() aren't the right tools for this, it's just the only idea that I have at the moment. Is there a better way?

Here is the stack trace:

[ August 01, 2008: Message edited by: Joseph Macer ]
 
Carey Evans
Ranch Hand
Posts: 225
Debian Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As Norm said, you must call an object's wait() method while you're synchronized on that object, or you will get an IllegalMonitorStateException; that's just how it works. The sender thread that calls wait() will release the lock and pause until another thread (with the lock) calls notify() or notifyAll(). The lock ensures that what you're waiting for doesn't change between checking for it, and calling wait().

There are (arguably) easier low-level ways to do locking now, like Semaphore, but I think you want a LinkedBlockingQueue instead of your ConcurrentLinkedQueue, which you wouldn't have to lock yourself at all.

A single-threaded Executor would be even easier.
 
Joseph Macer
Ranch Hand
Posts: 63
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Interesting! So with a LinkedBlockingQueue, I could add elements with offer(E e) WITHOUT using the synchronized keyword, and use poll(long timeout, TimeUnit unit) to make a thread wait (giving it an arbitrary large amount of time) until an element is available?

Is this thread-safe? Is this efficient?
[ August 02, 2008: Message edited by: Joseph Macer ]
 
Carey Evans
Ranch Hand
Posts: 225
Debian Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
LinkedBlockingQueue is thread-safe, and as efficient as practical. The time to do the I/O will be much greater than any operations on the queue, anyway.

I think you'd usually use the put() and take() methods, rather than offer() and poll().
 
Joseph Macer
Ranch Hand
Posts: 63
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Alright. My code will be a lot cleaner without synchronized everywhere
I will post again when I have my code working.
 
Henry Wong
author
Sheriff
Posts: 22534
109
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Be careful with mixing autoboxing and synchronization... See your following code:



The queueSize++ call is actually going to get unboxed, incremented, and boxed. This means that the object after the operation is *not* the same object as before the operation. This is probably not what you want when you synchronized on the object, because it now means that another thread can synchronize using the queueSize reference -- as it is referring to a different object.

Henry
 
Joseph Macer
Ranch Hand
Posts: 63
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Is this "unboxed" thing because I modify the variable with ++? How can I avoid that? Is this because it's a normal Integer, not designed to be thread-safe?

At any rate, I may not have to worry about keeping track of an integer size any more, because the ConcurrentLinkedQueue javadoc warns that size() is not a constant-time operation, but the suggested LinkedBlockingQueue contains no such warning. So I could just synchronize on the sendQueue object and use it's size() function.
 
Norm Radder
Bartender
Posts: 1526
14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you don't mind typing in a few more characters, don't use an Integer object. Make your own class that would allow you to change its contents without creating a new object as the boxing technique does when incrementing an Integer object. Then synchronized (queueSize) would work as expected.
 
Joseph Macer
Ranch Hand
Posts: 63
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Okay. It's obvious I don't understand enough about this sort of thing. I've been reading parts of "Java Threads, Third Edition" by Scott Oaks & Henry Wong, but a lot of their work seems to assume you've done multithreading in other languages. Are there other books or resources you could recommend?

Thanks for everyone's help
[ August 03, 2008: Message edited by: Joseph Macer ]
 
Carey Evans
Ranch Hand
Posts: 225
Debian Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you check the source for LinkedBlockingQueue, you can see that size() is constant-time because it's based on an AtomicInteger, which doesn't need synchronization.

In your original code, you could just use an int queueSize, and synchronize on the same object (sendQueue) when adding to or removing from the queue, and updating the count. Then you don't really need a ConcurrentLinkedQueue if you're locking anyway.

I just noticed you probably mean to start() your thread, not run() the sender in the calling thread.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!