Skip to content

Commit 854c925

Browse files
author
Matthew Sackman
committed
Better fix.
The ensureIsOpen() was there because a long time ago, we were starting the mainloop before enqueuing the startblocker. The mainloop could exit before the startblocker was enqueued, thus ensureIsOpen() was used to detect and explode should that occur. These days, we enqueue the startblocker before starting the mainloop. The problem was thus if the mainloop exploded really quickly, ensureIsOpen would collect the error, but if the mainloop was a little slower, the error would pop out when inspecting the result of the startblocker. These errors are of different types and were causing the problem seen. Because we enqueue the startblocker before we start the mainloop, we don't have to worry about the ensureIsOpen - if the mainloop dies, it'll call AMQConnection.shutdown which shutsdown channel0, which finds the current RPC and notifies that - that is the startblocker, so that's how the exception will always get back to the client user thread. Testing suggests that irregardless of whether the above waffle is correct or not, the problem has vanished.
1 parent a5ff08d commit 854c925

File tree

2 files changed

+0
-13
lines changed

2 files changed

+0
-13
lines changed

src/com/rabbitmq/client/impl/AMQConnection.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,14 +232,6 @@ public void start(boolean insist)
232232
ml.start();
233233

234234
try {
235-
// See bug 17389. The MainLoop could have shut down already in
236-
// which case we don't want to wait forever for a reply.
237-
238-
// There is no race if the MainLoop shuts down after enqueuing
239-
// the RPC because if that happens the channel will correctly
240-
// pass the exception into RPC, waking it up.
241-
ensureIsOpen();
242-
243235
AMQP.Connection.Start connStart =
244236
(AMQP.Connection.Start) connStartBlocker.getReply().getMethod();
245237

test/src/com/rabbitmq/client/test/BrokenFramesTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,6 @@ public void testNoMethod() throws Exception {
8282
conn.start(false);
8383
} catch (IOException e) {
8484
UnexpectedFrameError unexpectedFrameError = findUnexpectedFrameError(e);
85-
if (null == unexpectedFrameError) {
86-
Throwable cause = conn.getCloseReason().getCause();
87-
unexpectedFrameError = (UnexpectedFrameError)
88-
(cause instanceof UnexpectedFrameError ? cause : null);
89-
}
9085
assertNotNull(unexpectedFrameError);
9186
assertEquals(AMQP.FRAME_HEADER, unexpectedFrameError.getReceivedFrame().type);
9287
assertEquals(AMQP.FRAME_METHOD, unexpectedFrameError.getExpectedFrameType());

0 commit comments

Comments
 (0)