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

Prevent the potential concurrency hazard of the initialization in AbstractNioBoss(Worker)Pool.

Browse Source 
Motivation:
During the reading of the source codes of Netty 3.9.5.Final, I thought there might have a lurking concurrency bug in the AbstractNioBossPool#init method, of which a single volatile variable cannot correctly control the initialization sequence happened exactly only once under concurrency environment. Also I found there's already a much more elegant and concurrency safe way to do the work like this, I decided to make this PR. (Please refer to the discussion in https://github.com/netty/netty/issues/3249.)

Modifications:
Change the type of the variable that control the initialization from "volatile boolean" to "final AtomicBoolean".

Result:
The potential concurrency hazard of the initialization in AbstractNioBoss(Worker)Pool will be eliminated.
This commit is contained in:
luofucong 2014-12-16 15:15:29 +08:00 committed by Norman Maurer
parent ff9a6e0499
commit f0503ffb02
2 changed files with 6 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/main/java/org/jboss/netty/channel/socket/nio/AbstractNioBossPool.java
View File

@ -22,6 +22,7 @@ import org.jboss.netty.util.internal.ExecutorUtil;
import java.util.concurrent.Executor; import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
public abstract class AbstractNioBossPool<E extends Boss> public abstract class AbstractNioBossPool<E extends Boss>
@ -37,7 +38,7 @@ public abstract class AbstractNioBossPool<E extends Boss>
private final Boss[] bosses; private final Boss[] bosses;
private final AtomicInteger bossIndex = new AtomicInteger(); private final AtomicInteger bossIndex = new AtomicInteger();
private final Executor bossExecutor; private final Executor bossExecutor;
private volatile boolean initialized; private final AtomicBoolean initialized = new AtomicBoolean(false);
/** /**
* Create a new instance * Create a new instance
@ -66,10 +67,9 @@ public abstract class AbstractNioBossPool<E extends Boss>
} }
protected void init() { protected void init() {
if (initialized) { if (!initialized.compareAndSet(false, true)) {
throw new IllegalStateException("initialized already"); throw new IllegalStateException("initialized already");
} }
initialized = true;
for (int i = 0; i < bosses.length; i++) { for (int i = 0; i < bosses.length; i++) {
bosses[i] = newBoss(bossExecutor); bosses[i] = newBoss(bossExecutor);

7
src/main/java/org/jboss/netty/channel/socket/nio/AbstractNioWorkerPool.java
View File

@ -24,6 +24,7 @@ import org.jboss.netty.util.internal.ExecutorUtil;
import java.util.concurrent.Executor; import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
/** /**
@ -43,7 +44,7 @@ public abstract class AbstractNioWorkerPool<E extends AbstractNioWorker>
private final AbstractNioWorker[] workers; private final AbstractNioWorker[] workers;
private final AtomicInteger workerIndex = new AtomicInteger(); private final AtomicInteger workerIndex = new AtomicInteger();
private final Executor workerExecutor; private final Executor workerExecutor;
private volatile boolean initialized; private final AtomicBoolean initialized = new AtomicBoolean(false);
/** /**
* Create a new instance * Create a new instance
@ -71,12 +72,10 @@ public abstract class AbstractNioWorkerPool<E extends AbstractNioWorker>
} }
protected void init() { protected void init() {
if (initialized) { if (!initialized.compareAndSet(false, true)) {
throw new IllegalStateException("initialized already"); throw new IllegalStateException("initialized already");
} }
initialized = true;
for (int i = 0; i < workers.length; i++) { for (int i = 0; i < workers.length; i++) {
workers[i] = newWorker(workerExecutor); workers[i] = newWorker(workerExecutor);
} }