vert-x3 / vertx-amqp-client

An AMQP client for Vert.x
Apache License 2.0
17 stars 18 forks source link

Establish handler to deliver to before calling deliverMessageToHandler #42

Closed gemmellr closed 4 years ago

gemmellr commented 4 years ago

I noticed some further small issues while looking at #39.

The code makes various decisions under synchronization about whether to deliver, and then calls another method to actually do the delivery - which only then grabs the handler after re-synchronizing. Technically the handler might not be there any longer at this point, so it could NPE having removed the 'demand' already, and that message wouldn't be delivered later, a not dissimilar situation as the original report.

It seems simpler to just get the handler during the first synchronized area when deciding if there is any delivery to try or not. Avoids any such issues, and saves doing the second synchronization.

This also removes a redundant handler(..) call within the constructor; the null check will mean it does nothing, as the handler isnt set until after construction.

cescoffier commented 4 years ago

@vietj I've backported the fix into 3.9.

cescoffier commented 4 years ago

Thanks @gemmellr !