What do you think Andrzej? It seems like it does make sense.

IMHO this should go into the next bugfix release. It can cause message losing.

I think this is generally good idea but I would like to apply this fix after some tests to check if it works fine when more than one user resource is connected and if it is not causing packet duplication and so on (I do not see that it causes any of mentioned issues but it may).

Also I would like to work on message processing so it will not only store offline messages for sessions without presence, but same should be done for sessions with presence priority set to below 0 (-1 and so on).

Can any open please look into another issue of Offline messages?

https://projects.tigase.org/issues/2855

This patch may cause race condition in which one thread would check if session has presence set while checking messages sent from offline store while presence would not be set yet due to load on thread responsible for processing of presence. This would lead to messages being stored to offline store - same as in case described in issue #2855.

I wonder if this two issues are related.

Also this patch is only solution for OfflineMessages processor but I will not fix MessageAmp which I think has similar issue.

%kobit To make it simple we would need an API for plugins similar as API from #1601 which you would like to remove. Without this I would need to think about other way to fix this possible race condition and fix issue which attached patch should fix. I know you are against such an API so I will think about other solution as well as for solutions for other possible issues I mentioned in previous comment - but fixing this additional issues would be solution for #85 as well, should I proceed? If so please assign #85 to me.

Yes, Andrzej, please proceed.

However, I do not think we should be concerned about this particular race condition here (assuming I correctly understood what you mean). Please note the problem caused by the race condition is unavoidable anyway. This is because under a high load the user session can be unauthenticated or without a presence set (available or unavailable) while a packet is processed while the initial presence (available or unavailable) can be already in a queue. When the processing results are generated and sent, the presence packet can arrive and is processed.

The point is that the packets flow in a connection in both directions and state of the user session state can change on each end (client and SM) at any moment independently.

Our main concerns should be avoiding any data access conflicts and make sure that Tigase handles well all the border cases and events which are result of such session state changes.

I changed implementation of storing messages to offline storage so now messages will be stored to offline store until presence will be set.

In this change I also fixed handling of negative priority of resources.

Daniele, please let us know if it works OK for you.

Artur Hefczyc wrote:

Daniele, please let us know if it works OK for you.

Thanks guys, great work! The patch looks fine, however since we went live to production just a few days ago we won't be following the master branch anymore, but only stable releases. However, I have planned to create a test environment with a more "recent" Tigase version, but as you might see, my collaboration rate has decreased because of the Kontalk 3.0 release and a lot of other things. This means that the setup of our test environment might be delayed a bit.