| [leshan-dev] Concurrency Issue in QueuedRequestSender |
|
Hi all, we discovered a concurrency issue in the current queue mode implementation: In QueuedRequestSender send() method there gets checked if the client is still
registered in client registry (https://github.com/eclipse/leshan/blob/master/leshan-server-core/src/main/java/org/eclipse/leshan/server/queue/impl/QueuedRequestSender.java#L124). If the client is not registered we do not store and send this request to prevent having requests in the queue for no longer existing clients. Now if we assume two threads t1 (user thread that calls send()) and t2 (unregister thread that calls unregister()): - t1 passes the check mentioned above, but request was not added to messageStore yet - t2 removes all request from queue because client was unregistered - t1 adds the request to message store Now we have a request in the store without a corresponding registered client, i.e. this request may stay there forever (especially if we introduce persistence). We see different solutions for this issue. Because I am unsure which is the best, I am interested in your opinion. The solutions we see are the following: - Just do the above mentioned check after adding the request to the store. If the client is not registered anymore, remove the request again and do not send it. - Introduce a common lock in send() and unregister() method to ensure that no new requests get sent while unregister. We could use read/write locks there. - As we had multiple concurrency issues within queue mode already, introduce a thread model called "thread confinement", i.e. we execute all queue mode operations within one single thread. This may slow down execution, but makes maintenance and future extensions of code much easier. Are there any opinions out there for which solution we should go? Or does anyone have further ideas for a fix? We will provide a PR once we agreed for a solution. Best Regards, Daniel |