andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix improper synchronization in DefaultPromise. Fixes #5489

Browse Source 
Motivation:

A race detector found that DefaultPromise.listeners is improperly synchronized [1].
Worst case a listener will not be executed when the promise is completed.

Modifications:

Make DefaultPromise.listeners a volatile.

Result:

Hopefully, DefaultPromise is more correct under concurrent execution.

[1] https://github.com/grpc/grpc-java/issues/2015
This commit is contained in:
buchgr 2016-07-04 09:49:09 +02:00 committed by Norman Maurer
parent be3e6972a1
commit b0a5d4c266
1 changed files with 1 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
common/src/main/java/io/netty/util/concurrent/DefaultPromise.java
View File

@ -60,7 +60,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
* *
* Threading - synchronized(this). We must support adding listeners when there is no EventExecutor. * Threading - synchronized(this). We must support adding listeners when there is no EventExecutor.
*/ */
private Object listeners; private volatile Object listeners;
/** /**
* Threading - synchronized(this). We are required to hold the monitor to use Java's underlying wait()/notifyAll(). * Threading - synchronized(this). We are required to hold the monitor to use Java's underlying wait()/notifyAll().
*/ */
@ -417,7 +417,6 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
} }
private void notifyListeners() { private void notifyListeners() {
// Modifications to listeners should be done in a synchronized block before this, and should be visible here.
if (listeners == null) { if (listeners == null) {
return; return;
} }