0

I want to store a list of objects in a thread safe manner, while maintaining priority. Originally I started out using a BlockingQueue for this as it's thread safe and has ability to maintain custom priority.

I'm wondering if I need to synchronize my methods? My code looks like:

void addToQueue(SomeObject obj) {
    ... put it on my priority queue
    ... do some logging
}

What I have noticed is the logging is happening out of order, when accessing addToQueue from multiple threads. So I wrapped my method like so:

void addToQueue(SomeObject obj) {
    syncronized(myMutex) {
        ... put it on my priority queue
        ... do some logging
    }
}

This seemed to keep the logging in order. So now I've come to the conclusion that if I'm going this route, then maybe my code would be more efficient by not using a BlockingQueue but instead use a Set or List and manage the priority myself.

Maybe I have some misunderstanding of the BlockingQueue.

Peter Millan
  • 3
  • 2
  • 2
    The only thing synchronized in a `BlockingQueue` is a part of the code in the individual methods. Logging is not part of the `BlockingQueue` and therefore is not tied to its execution. – Sotirios Delimanolis Oct 21 '13 at 20:46
  • Is that an Object a Collection? The bulk Collection operations addAll, containsAll, retainAll and removeAll are not necessarily performed atomically unless specified otherwise in an implementation. So it is possible, for example, for addAll(c) to fail (throwing an exception) after adding only some of the elements in c. http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/BlockingQueue.html – JNL Oct 21 '13 at 20:48
  • By nature, the BlockingQueue has blocking operations, so be careful about making a blocking call inside a synchronized section of code otherwise you might end up in a deadlock. – jekennedy Oct 28 '13 at 15:47

2 Answers2

1

A Queue that additionally supports operations that wait for the queue to become non-empty when retrieving an element, and wait for space to become available in the queue when storing an element.

This is the javadoc for BlockingQueue. You use it if you need this blocking behavior, otherwise you don't.

BlockingQueue does not maintain any priority, it is strictly first-in-first-out. Perhaps you are using PriorityBlockingQueue?

Coming to your pseudocode:

void addToQueue(SomeObject obj) {
    ... put it on my priority queue
    ... do some logging
}

The queue is thread-safe, but that only means that multiple threads can concurrently call put it on my priority queue without any data corruption. It does not guarantee any of the following:

  • If there are multiple threads blocked which one will succeed first
  • If a thread X completes the put before a thread Y then thread X will also complete the logging before thread Y.

If you need all of addToQueue occur without interleaving from other threads then you need to synchronize. Note that you can use the queue object itself:

void addToQueue(SomeObject obj) {
    synchronized (queue) {
        ... put it on my priority queue
        ... do some logging
    }
}
Miserable Variable
  • 28,432
  • 15
  • 72
  • 133
0

http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/BlockingQueue.html

As with other concurrent collections, actions in a thread prior to placing an object into a BlockingQueue happen-before actions subsequent to the access or removal of that element from the BlockingQueue in another thread.

If you want to use that safety to get ordered logging you have to log before putting items into the queue and after taken an item from the queue.

I wouldn’t use synchronized for getting ordered logging. Multi-threading means parallel execution and that implies that certain actions don’t have an ordering. Log Records can have a time-stamp and seeing them in the wrong order, i.e. in the console, looks like a minor glitch to me that is not worth sacrificing the advantages of parallel execution for.

Holger
  • 285,553
  • 42
  • 434
  • 765