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

* Added detailed explanation on NETTY-114 (not to forget why I woke up selector again)

Browse Source
This commit is contained in:
Trustin Lee 2009-06-11 16:47:55 +00:00
parent e9d07c3c2e
commit 0a5019385c
3 changed files with 84 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/main/java/org/jboss/netty/channel/socket/nio/NioClientSocketPipelineSink.java
View File

@ -233,9 +233,34 @@ class NioClientSocketPipelineSink extends AbstractChannelSink {
try { try {
int selectedKeyCount = selector.select(500); int selectedKeyCount = selector.select(500);
// Wake up immediately in the next turn if someone might // 'wakenUp.compareAndSet(false, true)' is always evaluated
// have waken up the selector between 'wakenUp.set(false)' // before calling 'selector.wakeup()' to reduce the wake-up
// and 'selector.select(...)'. // overhead. (Selector.wakeup() is an expensive operation.)
//
// However, there is a race condition in this approach.
// The race condition is triggered when 'wakenUp' is set to
// true too early.
//
// 'wakenUp' is set to true too early if:
// 1) Selector is waken up between 'wakenUp.set(false)' and
// 'selector.select(...)'. (BAD)
// 2) Selector is waken up between 'selector.select(...)' and
// 'if (wakenUp.get()) { ... }'. (OK)
//
// In the first case, 'wakenUp' is set to true and the
// following 'selector.select(...)' will wake up immediately.
// Until 'wakenUp' is set to false again in the next round,
// 'wakenUp.compareAndSet(false, true)' will fail, and therefore
// any attempt to wake up the Selector will fail, too, causing
// the following 'selector.select(...)' call to block
// unnecessarily.
//
// To fix this problem, we wake up the selector again if wakenUp
// is true immediately after selector.select(...).
// It is inefficient in that it wakes up the selector for both
// the first case (BAD - wake-up required) and the second case
// (OK - no wake-up required).
if (wakenUp.get()) { if (wakenUp.get()) {
selector.wakeup(); selector.wakeup();
} }

31
src/main/java/org/jboss/netty/channel/socket/nio/NioUdpWorker.java
View File

@ -224,9 +224,34 @@ class NioUdpWorker implements Runnable {
try { try {
int selectedKeyCount = selector.select(500); int selectedKeyCount = selector.select(500);
// Wake up immediately in the next turn if someone might // 'wakenUp.compareAndSet(false, true)' is always evaluated
// have waken up the selector between 'wakenUp.set(false)' // before calling 'selector.wakeup()' to reduce the wake-up
// and 'selector.select(...)'. // overhead. (Selector.wakeup() is an expensive operation.)
//
// However, there is a race condition in this approach.
// The race condition is triggered when 'wakenUp' is set to
// true too early.
//
// 'wakenUp' is set to true too early if:
// 1) Selector is waken up between 'wakenUp.set(false)' and
// 'selector.select(...)'. (BAD)
// 2) Selector is waken up between 'selector.select(...)' and
// 'if (wakenUp.get()) { ... }'. (OK)
//
// In the first case, 'wakenUp' is set to true and the
// following 'selector.select(...)' will wake up immediately.
// Until 'wakenUp' is set to false again in the next round,
// 'wakenUp.compareAndSet(false, true)' will fail, and therefore
// any attempt to wake up the Selector will fail, too, causing
// the following 'selector.select(...)' call to block
// unnecessarily.
//
// To fix this problem, we wake up the selector again if wakenUp
// is true immediately after selector.select(...).
// It is inefficient in that it wakes up the selector for both
// the first case (BAD - wake-up required) and the second case
// (OK - no wake-up required).
if (wakenUp.get()) { if (wakenUp.get()) {
selector.wakeup(); selector.wakeup();
} }

31
src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java
View File

@ -162,9 +162,34 @@ class NioWorker implements Runnable {
try { try {
int selectedKeyCount = selector.select(500); int selectedKeyCount = selector.select(500);
// Wake up immediately in the next turn if someone might // 'wakenUp.compareAndSet(false, true)' is always evaluated
// have waken up the selector between 'wakenUp.set(false)' // before calling 'selector.wakeup()' to reduce the wake-up
// and 'selector.select(...)'. // overhead. (Selector.wakeup() is an expensive operation.)
//
// However, there is a race condition in this approach.
// The race condition is triggered when 'wakenUp' is set to
// true too early.
//
// 'wakenUp' is set to true too early if:
// 1) Selector is waken up between 'wakenUp.set(false)' and
// 'selector.select(...)'. (BAD)
// 2) Selector is waken up between 'selector.select(...)' and
// 'if (wakenUp.get()) { ... }'. (OK)
//
// In the first case, 'wakenUp' is set to true and the
// following 'selector.select(...)' will wake up immediately.
// Until 'wakenUp' is set to false again in the next round,
// 'wakenUp.compareAndSet(false, true)' will fail, and therefore
// any attempt to wake up the Selector will fail, too, causing
// the following 'selector.select(...)' call to block
// unnecessarily.
//
// To fix this problem, we wake up the selector again if wakenUp
// is true immediately after selector.select(...).
// It is inefficient in that it wakes up the selector for both
// the first case (BAD - wake-up required) and the second case
// (OK - no wake-up required).
if (wakenUp.get()) { if (wakenUp.get()) {
selector.wakeup(); selector.wakeup();
} }