Reputation: 267040
Say I have a class:
public class Chat {
private volatile ConcurrentLinkedQueue messages = new ConcurrentLinkedQueue();
// getter/setter for messages queue
}
And I have a background thread that takes an instance of this class as a parameter:
Thread t = new Thread(new QueuePersister(messages));
t.start();
Where thread's task is:
public class QueuePersister implements Runnable {
private volatile ConcurrentLinkedQueue messages = new ConcurrentLinkedQueue();
public QueuePersister(ConcurrentLinkedQueue messages) {
this.messages = messages;
}
@Override
public void run() {
while(true) {
// this is a 2 step process, probably should synchronize?? i.e. copy and re-initializing
ConcurrentLinkedQueue copy = messages;
messages = new ConcurrentLinkedQueue();
// save to disk using the copy queue
// sleep for x seconds
}
}
}
The idea I am trying to do is:
My messages are saved to the queue, and every x seconds a background thread makes a copy of the queue, re-sets the original message queue so it can start taking new data while the old copy gets persisted to file/db.
This way any future writes will be done to a new queue.
In my testing, this doesn't work because I can't seem to be able to re-initialize the queue that was passed into the thread.
I believe this is because the messages queue gets passed in by reference, but it is passing a copy of the reference and you are not allowed to change the reference. You are allowed to change the object that is being referred to, but not change the reference.
If this is true, what options do I have to do this then? Can I expose some methods at on the class Chat that will do this for me?
Note: when my application is running, the Chat object is created only once.
The Chat object will be accessed by multiple threads.
Update
There will only be a single thread that will ever be doing this "Persistance", and I want it to work on the Chat.messages queue. What I want it to do is simply make a copy of the collection, and re-set the Chat's collection and then it can take its time to persist the copied version of the queue to disk.
Upvotes: 0
Views: 680
Reputation: 116888
So the messages
inside of your thread is a different messages
in the Chat
class right? So when you do:
ConcurrentLinkedQueue copy = messages;
messages = new ConcurrentLinkedQueue();
this is only affecting the Thread
collection field. You don't need that to be volatile
nor does it need to be concurrent.
What I suspect you are trying to do is to consume the collection in the thread. Since you are using a ConcurrentLinkedQueue
, you can make operations on the queue from the other thread without having to do the copy and replace dance. You can remove items from the queue safely while other threads are adding to the queue. That's the whole purpose of the concurrent classes.
You should mark the messages
field in Chat
to be private final
and not volatile since it doesn't need to change. You can also mark it final inside of the Thread
.
Upvotes: 3
Reputation: 40256
I would consider using a LinkedBlockingQueue instead. It has a drainTo
method that will empty the contents of the queue into another. At this point you can declare the LinkedBlockingQueue as final.
Javadoc
public int drainTo(Collection c)
Removes all available elements from this queue and adds them to the given collection. This operation may be more efficient than repeatedly polling this queue. A failure encountered while attempting to add elements to collection c may result in elements being in neither, either or both collections when the associated exception is thrown. Attempts to drain a queue to itself result in IllegalArgumentException. Further, the behavior of this operation is undefined if the specified collection is modified while the operation is in progress.
Upvotes: 1