RocksDbQueue Threading Tweaks #940
Conversation
…omicLong. Synchronize close method.
| private final Set<RocksDbTask<T>> outstandingTasks = new HashSet<>(); | ||
|
|
||
| private final AtomicBoolean closed = new AtomicBoolean(false); | ||
| private boolean closed = false; |
There was a problem hiding this comment.
Do these vars need to be volatile?
There was a problem hiding this comment.
No because every access to them is within a synchronized section.
There was a problem hiding this comment.
Still getting up to speed on threading obviously :) . I didn't realize synchronized causes all the vars to be updated from main memory.
There was a problem hiding this comment.
Yep, it has a read-fence on entry to the synchronized block and a write one on exit. So as long as both the writing thread and the reading thread used synchronized there's no need for volatile.
| private final Set<RocksDbTask<T>> outstandingTasks = new HashSet<>(); | ||
|
|
||
| private final AtomicBoolean closed = new AtomicBoolean(false); | ||
| private boolean closed = false; |
There was a problem hiding this comment.
Still getting up to speed on threading obviously :) . I didn't realize synchronized causes all the vars to be updated from main memory.
PR description
Since
RocksDbQueueusessynchronizedfor its methods, it doesn't need to also useAtomicLongandAtomicBoolean. Theclosemethod wasn't synchronized but needs to be since it has to access thedequeueIteratorfield to close it.