Skip to content

Fix: replace channel semaphore with a lock - #135

Open
678098 wants to merge 2 commits into
bloomberg:mainfrom
678098:260904_connection_lock
Open

Fix: replace channel semaphore with a lock#135
678098 wants to merge 2 commits into
bloomberg:mainfrom
678098:260904_connection_lock

Conversation

@678098

@678098 678098 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fix possible indefinite wait on waitUntilWritable: closed channel never freed a semaphore that we wait for in waitUntilWritable.

Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
@678098
678098 requested a review from a team as a code owner September 4, 2026 15:46
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
assertFalse(waiter.isAlive());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fails in main:

[ERROR] com.bloomberg.bmq.it.NettyTcpConnectionImplIT.testWaitUntilWritableReturnsWhenChannelGoesDown -- Time elapsed: 31.59 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
	at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:257)
	at com.bloomberg.bmq.it.NettyTcpConnectionImplIT.testWaitUntilWritableReturnsWhenChannelGoesDown(NettyTcpConnectionImplIT.java:1060)

@678098 678098 changed the title IT[NettyTcpConnectionImplIT]: add is writeable test Fix: release semaphore on channel close Sep 4, 2026
@678098
678098 force-pushed the 260904_connection_lock branch from e529193 to 6ecb6f2 Compare September 4, 2026 16:56
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
@678098
678098 force-pushed the 260904_connection_lock branch from 6ecb6f2 to 01c4686 Compare September 4, 2026 17:08
// The channel is gone, so it can never become writable again.
// Release the threads waiting for that to happen; their writes
// complete with a not-connected result.
lock.notifyAll();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code hasn't got any semaphore notification during channel close.
This meant that a thread that is blocked in waitUntilWritable will never be unblocked if a channel was closed.

We don't even need a separate semaphore to send this notification: existing lock is enough.

@678098 678098 changed the title Fix: release semaphore on channel close Fix: replace channel semaphore with a lock Sep 8, 2026
@678098
678098 requested a review from pniedzielski September 8, 2026 15:16
@678098 678098 assigned pniedzielski and 678098 and unassigned pniedzielski Sep 8, 2026
@678098

678098 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

@pniedzielski I am going to add a few more fixes to this PR, will reassign once it is ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants