ResponseRouter.close never actually called caused message lost

[AngryGami]

JMSQueue.request method do:

...
ResponseRouter.routerFor(this, codecs, routerOpts).registerResponse(id, response);
...

And when actual queue get closed associated router not get closed ever because link to created router is not stored anywhere. It probably should be added to "closeables" for this queue by doing something like:

ResponseRouter router = ResponseRouter.routerFor(this, codecs, routerOpts);
addCloseable(router);
router.registerResponse(id, response);

In either case when queue get restarted (without JVM restart) with new listener - this new listener will never get called (because router is pointing to old listener).

[tobias]

@AngryGami - thanks for this report, and thanks for tracking down the cause! Are you seeing this with TorqueBox or Immutant (the bug should exist in both places, I'm just curious)?

[AngryGami]

np, :) I see this problem on Immutant 2.1.0. (was not there on Immutant 2.0.0 though)

[tobias]

Thanks. I'll see if I can get you a fix in an incremental build today to test.

[tobias]

It looks like we are registering the router as a closeable on the queue: https://github.com/projectodd/wunderboss/blob/master/modules/messaging/src/main/java/org/projectodd/wunderboss/messaging/ResponseRouter.java#L67

Let me see if I can recreate the issue. Do you have a snippet of code that recreates the problem? That would help me by making sure I'm seeing the same issue you are.

[tobias]

Looks like I can recreate it with the following (the assertion in the second action call fails):

(deftest request-receive-should-work-with-recreated-queue
  (let [q (random-queue)
        action (fn [q msg]
                 (respond q identity)
                 (is (= msg (deref (request q msg) 1000 :failure)))
                 (stop q))]
    (action q :hi)
    (action (queue (.name q)) :hi-again)))

Does that test match what you are doing?

[tobias]

I've found the issue - we are no longer closing the registered closeables when the destination is stopped. I've created https://issues.jboss.org/browse/IMMUTANT-583 to track this, and have a fix that I'll push soon, and will have a incremental shortly thereafter.

[AngryGami]

Yep, that is pretty much what I was doing. Apparently I did trace problem but not to real cause :) - missed call to addCloseable inside ResponseRouter. I wounder if the problem lies as you said in "we are no longer closing the registered closeables" - it might have major impact on the system like resource/memory leakages

[tobias]

Correct - the issue definitely is that we are no longer closing the closeables. However, it would only have resource leakage impact if you are dynamically starting/stopping many destinations at runtime outside of WildFly, or doing lots of redeploys with WildFly w/o restarting WildFly itself. That being said, it's definitely something important to fix, and will likely be a primary motivator for a 2.1.1 release.

[AngryGami]

"dynamically starting/stopping many destinations at runtime outside of WildFly" - exactly my usecase :)) Though "many" is not the case - around 10 maybe.

[tobias]

Can you give incremental build #653 a try? It should have this fixed (please see http://immutant.org/builds/2x/ if you need guidance on using incremental builds).

[AngryGami]

Yep, build 653 works.

[tobias]

Good deal. I think we'll get a 2.1.1 out towards the end of the month that will include this fix.

[tobias]

This was fixed in Immutant 2.1.1, so closing.