ansd opened a new pull request, #76: URL: https://github.com/apache/qpid-jms/pull/76
A consumer created on a connection that has not been started was granted its full prefetch of link credit straight away, in the flow frame that follows its attach. The remote is then free to send, and it does: the messages are transferred on that consumer's link and buffered client side, where the application cannot see them until Connection.start() is called. [Jakarta Messaging 6.1.4](https://jakarta.ee/specifications/messaging/3.1/jakarta-messaging-spec-3.1#starting-a-connection) says a connection is created in stopped mode, "that means that no messages are being delivered to it", and is emphatic about the guarantee: "clients rely on the fact that no messages will be delivered to a consumer until its connection has been started. Jakarta Messaging Providers must ensure that this is the case." Granting credit before start defeats that. The messages have been delivered to the consumer in every sense the protocol recognises - transferred on its link, and held as unsettled deliveries owned by it at the peer. Only the last hop, into the application, is withheld. That is not merely a semantic quibble, because delivery is exclusive. Once a message has been transferred to one consumer's link it is no longer available to any other consumer on that queue. A consumer on a connection that is never started therefore takes messages that a started consumer on the same queue can never receive. If that connection stays open, the messages are stranded for its lifetime, invisible to every application. This is what the Jakarta Messaging TCK trips over in core20/jmsconsumertests. JmsTool's queue setup creates a connection, a session, a producer and a consumer, and never starts that connection; the test then opens a JMSContext of its own, whose consumer is started, sends one message and blocks in receive(). Both links hold credit, so the peer picks one, and the outcome depends on which - a detail no specification defines. Against RabbitMQ, which selects in attach order, the never-started consumer attached first and always won, so the started consumer blocked forever and three tests hung until killed. Swapping the attach order alone makes the same test pass, which shows how thin the ice is: brokers that select differently pass today with this bug present and unfixed. So grant credit at the point where the specification allows delivery to begin. JmsMessageConsumer.init() now only starts the consumer resource if the session is already started, and JmsSession.start() starts the resource for the consumers that were created while it was stopped. Both paths are needed: JmsSession.start() is once-only and iterates the consumers that exist at the time, so a consumer created after the connection was started - which 6.1.4 explicitly permits - would otherwise never be credited. Between them each consumer is credited exactly once, at the right moment. The pairing of JmsMessageConsumer.start() with startConsumerResource() is the one resumeAfterRollback() already uses for the same purpose. Two existing tests asserted the previous behaviour directly, expecting credit and inbound transfers on a connection that was never started, and checking only that receive() returned null. What they are really about is that the application does not see messages while delivery is paused, which is equally true of a connection that has been stopped, so they now start the connection, let the messages arrive, and stop it before asserting. They are renamed to testNoReceivedMessagesWhenConnectionStopped and testNoReceivedNoWaitMessagesWhenConnectionStopped. Twenty other tests created a consumer on an unstarted connection incidentally, for reasons that have nothing to do with delivery, and simply start the connection now. Connection.stop() has the same shape of problem and is not addressed here: it stops the client side message queue but leaves the link credit in place, so a stopped connection's consumer also goes on taking messages from its queue. Fixing that means draining credit and waiting out the in-flight deliveries, as suspendForRollback() does, and is left for separate work. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
