Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SharpXMPP thread safety #78

Open
ForNeVeR opened this issue Sep 1, 2019 · 0 comments
Open

SharpXMPP thread safety #78

ForNeVeR opened this issue Sep 1, 2019 · 0 comments
Assignees

Comments

@ForNeVeR
Copy link
Member

ForNeVeR commented Sep 1, 2019

There's a hot discussion happening between our glorious Dr. @ForNeVeR and his mere servant, @ForNeVeR, in scope of #18:

@ForNeVeR please investigate thread safety of our current solution and raise another issue about that. Probably we'll need to solve that first, but this is an issue about XMPP connection/room session API.

Thanks for the question, I didn't initially thought about that.

The SharpXMPP itself is thread-safe if the library client doesn't allow multiple simultaneous calls to the Send method.

So the architecture proposed here should be thread-safe as well. Our current usage is not thread safe.

Further analysis has shown that not to be true. Sometimes, SharpXMPP will send its own data to the server (e.g. when answering some IQ queries). Looks like it isn't safe by design. Please investigate more and fix.

So, SharpXMPP has some issues. Here's what could happen:

  1. Our thread sends a message through SharpXMPP (I believe, we use public methods that just call to XmppClient.NetworkStream.Write or something like that)
  2. At the same moment, an external entity may send us an IQ query (e.g. ask for a client identification data)
  3. SharpXMPP will do its best to answer the IQ query immediately (thus occupying the same NetworkStream.Write call)

In case the NetworkStream.Write isn't thread-safe for this scenario (e.g. will overlap two pieces of data), everything will go to hell. So, Mr. @ForNeVeR, I fear you'll have to:

  1. Investigate if the hypothesis is right:
    • is NetworkStream (or whatever we use for message sending) really not thread safe?
    • do we really use it is an unsafe manner together with SharpXMPP?
  2. If the problem really exists, then
    • write a test that will reproduce the issue (should be possible with current SharpXMPP API)
    • deside if it's possible to fix without changing SharpXMPP code
    • fix the issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant