Skip to content

Commit 583fc27

Browse files
authored
Fixed|SimpleChannelPool.close() should only return after complete. (#7927)
Motivation: We need to ensure we only return from close() after all work is done as otherwise we may close the EventExecutor before we dispatched everything. Modifications: Correctly wait on operations to complete before return. Result: Fixes #7901.
1 parent 88f0586 commit 583fc27

File tree

2 files changed

+39
-21
lines changed

2 files changed

+39
-21
lines changed

transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java

+37-20
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import io.netty.util.concurrent.EventExecutor;
2121
import io.netty.util.concurrent.Future;
2222
import io.netty.util.concurrent.FutureListener;
23+
import io.netty.util.concurrent.GlobalEventExecutor;
2324
import io.netty.util.concurrent.Promise;
2425
import io.netty.util.internal.ObjectUtil;
2526
import io.netty.util.internal.ThrowableUtil;
@@ -437,27 +438,43 @@ public void acquired() {
437438

438439
@Override
439440
public void close() {
440-
executor.execute(new Runnable() {
441-
@Override
442-
public void run() {
443-
if (!closed) {
444-
closed = true;
445-
for (;;) {
446-
AcquireTask task = pendingAcquireQueue.poll();
447-
if (task == null) {
448-
break;
449-
}
450-
ScheduledFuture<?> f = task.timeoutFuture;
451-
if (f != null) {
452-
f.cancel(false);
453-
}
454-
task.promise.setFailure(new ClosedChannelException());
455-
}
456-
acquiredChannelCount = 0;
457-
pendingAcquireCount = 0;
458-
FixedChannelPool.super.close();
441+
if (executor.inEventLoop()) {
442+
close0();
443+
} else {
444+
executor.submit(new Runnable() {
445+
@Override
446+
public void run() {
447+
close0();
448+
}
449+
}).awaitUninterruptibly();
450+
}
451+
}
452+
453+
private void close0() {
454+
if (!closed) {
455+
closed = true;
456+
for (;;) {
457+
AcquireTask task = pendingAcquireQueue.poll();
458+
if (task == null) {
459+
break;
460+
}
461+
ScheduledFuture<?> f = task.timeoutFuture;
462+
if (f != null) {
463+
f.cancel(false);
459464
}
465+
task.promise.setFailure(new ClosedChannelException());
460466
}
461-
});
467+
acquiredChannelCount = 0;
468+
pendingAcquireCount = 0;
469+
470+
// Ensure we dispatch this on another Thread as close0 will be called from the EventExecutor and we need
471+
// to ensure we will not block in a EventExecutor.
472+
GlobalEventExecutor.INSTANCE.execute(new Runnable() {
473+
@Override
474+
public void run() {
475+
FixedChannelPool.super.close();
476+
}
477+
});
478+
}
462479
}
463480
}

transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,8 @@ public void close() {
394394
if (channel == null) {
395395
break;
396396
}
397-
channel.close();
397+
// Just ignore any errors that are reported back from close().
398+
channel.close().awaitUninterruptibly();
398399
}
399400
}
400401
}

0 commit comments

Comments
 (0)