REST - Race condition in QueueDestinationsResource.findQueue

[amdonov]

There is race condition at the start of the findQueue method of QueueDestinationsResource in REST component. I discovered the problem when blasting requests at a server that I'd just restarted. When I posted a single message at start up, a single single post consumer was started for each registered subscriber. After that, I could submit any number of requests and no more post consumers were started. However, if I hit the server with 25 requests right away, it started up 25 post consumers. In looking at the code, there is a check then act situation, findQueue checks to see if the queue has already been initialized. If not, it starts it. However, multiple threads hit this method at the same time result in the queue be initialized multiple times. Not sure the best way to handle this short of synchronizing on queues or initializing them in advance.

[jbertram]

https://github.com/hornetq/hornetq/pull/734

[amdonov]

Justin,

Thanks for looking at this. I took a slightly different approach in my code that you might want to consider (see below), but I didn't post my fix because I think that lazy initialization of the consumers causes other problems, and I haven't had a chance investigate a better solution. Consider the case where 1000 messages are posted, and durable consumers are working through the backlog. If the server is restarted after 100 messages have been processed, the 900 other messages in the queue will just sit there until another message is posted. This seems like bad behavior. I think that it would be better if durable consumers were started at server startup rather than waiting for a REST call to trigger initialization.

If you still want to lazy initialize may I suggest that rather than synchronize the entire findQueue method, just synchronize on queues if queue == null. It does require a second null check inside the sychronized block because another thread could have completed initialization while the blocking thread waited for a lock. However, it does avoid sychronization for the majority of the calls to findQueue.

What are you thoughts?

Aaron