Deadlock

[dsparkplug]

I've found an circular locking issue with the IStanzaEventListener methods in the XmppStream class .

Basically the IStanzaEventListener.StanzaReceived which acquires a [MethodImpl(MethodImplOptions.Synchronized)] lock can call IStanzaEventListener.BytesWritten which also needs to acquire a [MethodImpl(MethodImplOptions.Synchronized)] lock and therefore deadlocks.

The chain of calls is as follows:

XmppStream.StanzaReceived acquires a MethodImpl(MethodImplOptions.Synchronized) lock --> calls XmppStream.OnElement if (State is not ServerFeaturesState.Instance or SASLState.Instance or StartTLSState.Instance or CompressionState.Instance or SASLAuthedState.Instance) --> invokes XmppStream.OnProtocol event In IQTracker OnProtocol event is set to OnIQ --> calls IQTracker.Call which calls the 'cb' callback which is set to JabberClient.GotSession in JabberClient.GotResource --> sets JabberClient.IsAuthenticated --> calls JabberClient.Presence --> calls JabberClient.Write --> calls SocketStanzaStream.JabberClient.Write --> calls AsyncSocket.Write --> calls Stream.BeginWrite with callback WroteData --> WroteData calls SocketStanzaStream.OnWrite --> calls XmppStream.BytesWritten which waits for a MethodImpl(MethodImplOptions.Synchronized) lock

Any ideas how to go about fixing this? The way this code uses callbacks is so convoluted its hard to know where to start without breaking something else.

The chain of events and subsequent deadlock is reproducible when sending a large number of XMPP messages.

[ForNeVeR]

Alright, that's a fair question.

Do you have any repro scenario? It would be easier to check (or even to write a unit test) when we have a MCVE.

I still didn't understood what thread exactly deadlock here, so I'm not sure how to proceed.

I have an idea to make more things async / threadsafe by default in our code, but first I need to better understand this particular case.

I agree that the code base isn't in the best state now. Hopefully I'll finish some big things in my life in a couple of months and will be able to come back to this library.

[KotM]

In our project we have encountered with similar issues on simultaneous XmppStream.Write() attempts. At the least application waits a response from XmppStream.Write() from several threads. It seems to be a deadlock.

Currently I'll try to update our code by adding simply lock clause around this command, hope it will help.

[KotM]

Unfortunately, locking doesn't help. Currently we're captured XmppStream.Write() hanging again, with lock state. Find below a stack trace (this thread was stopped on XmppStream.Write(), another one on lock state):

    [In a sleep, wait, or join] 
    JabberNet.dll!JabberNet.jabber.connection.XmppStream.JabberNet.jabber.connection.IStanzaEventListener.BytesWritten(byte[] buf = {byte[1437]}, int offset = 0, int count = 1437) + 0x3f bytes    
    JabberNet.dll!JabberNet.bedrock.net.AsyncSocket.WroteData(System.IAsyncResult ar) + 0xbf bytes  
    System.dll!System.Net.LazyAsyncResult.Complete(System.IntPtr userToken) + 0x52 bytes    
    System.dll!System.Net.ContextAwareResult.CaptureOrComplete(ref System.Threading.ExecutionContext cachedContext, bool returnContext) + 0xfb bytes    
    System.dll!System.Net.ContextAwareResult.FinishPostingAsyncOp(ref System.Net.CallbackClosure closure = {System.Net.CallbackClosure}) + 0x77 bytes   
    System.dll!System.Net.Sockets.Socket.BeginSend(byte[] buffer, int offset, int size, System.Net.Sockets.SocketFlags socketFlags, out System.Net.Sockets.SocketError errorCode, System.AsyncCallback callback, object state) + 0x11c bytes    
    System.dll!System.Net.Sockets.Socket.BeginSend(byte[] buffer, int offset, int size, System.Net.Sockets.SocketFlags socketFlags, System.AsyncCallback callback, object state) + 0x41 bytes   
    System.dll!System.Net.Sockets.NetworkStream.BeginWrite(byte[] buffer, int offset, int size, System.AsyncCallback callback, object state) + 0xa4 bytes   
    JabberNet.dll!JabberNet.bedrock.net.AsyncSocket.Write(byte[] buf, int offset, int len) + 0x162 bytes    
    JabberNet.dll!JabberNet.jabber.connection.SocketStanzaStream.Write(string str) + 0x92 bytes 

[our code].Utilities.Jabber.PracticeAreaChannel.RespondToQuery(JabberNet.jabber.protocol.client.IQ response = Cannot evaluate expression because the current thread is in a sleep, wait, or join) Line 914 + 0x22 bytes C#

[our stack here, removed by security reasons]

    System.dll!System.Net.LazyAsyncResult.Complete(System.IntPtr userToken) + 0x52 bytes    
    mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) + 0x15e bytes 
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) + 0x17 bytes  
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) + 0x52 bytes    
    System.dll!System.Net.ContextAwareResult.Complete(System.IntPtr userToken) + 0x98 bytes 
    System.dll!System.Net.LazyAsyncResult.ProtectedInvokeCallback(object result, System.IntPtr userToken) + 0xcc bytes  
    System.dll!System.Net.HttpWebRequest.ProcessResponse() + 0x13c bytes    
    System.dll!System.Net.HttpWebRequest.SetResponse(System.Net.CoreResponseData coreResponseData) + 0x1b2 bytes    
    System.dll!System.Net.ConnectionReturnResult.SetResponses(System.Net.ConnectionReturnResult returnResult = {System.Net.ConnectionReturnResult}) + 0xbb bytes    
    System.dll!System.Net.Connection.ReadComplete(int bytesRead, System.Net.WebExceptionStatus errorStatus) + 0x450 bytes   
    System.dll!System.Net.Connection.PostReceive() + 0x109 bytes    
    System.dll!System.Net.Connection.ReadComplete(int bytesRead, System.Net.WebExceptionStatus errorStatus) + 0x2d1 bytes   
    System.dll!System.Net.LazyAsyncResult.Complete(System.IntPtr userToken) + 0x52 bytes    
    System.dll!System.Net.LazyAsyncResult.ProtectedInvokeCallback(object result, System.IntPtr userToken) + 0xcc bytes  
    System.dll!System.Net.Security._SslStream.ProcessFrameBody(int readBytes, byte[] buffer, int offset, int count, System.Net.AsyncProtocolRequest asyncRequest) + 0x14b bytes 
    System.dll!System.Net.Security._SslStream.ReadFrameCallback(System.Net.AsyncProtocolRequest asyncRequest = {System.Net.AsyncProtocolRequest}) + 0xab bytes  
    System.dll!System.Net.AsyncProtocolRequest.CompleteRequest(int result) + 0x3d bytes 
    System.dll!System.Net.FixedSizeReader.CheckCompletionBeforeNextRead(int bytes) + 0x29 bytes 
    System.dll!System.Net.FixedSizeReader.ReadCallback(System.IAsyncResult transportResult) + 0x78 bytes    
    System.dll!System.Net.LazyAsyncResult.Complete(System.IntPtr userToken) + 0x52 bytes    
    mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) + 0x15e bytes 
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) + 0x17 bytes  
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) + 0x52 bytes    
    System.dll!System.Net.ContextAwareResult.Complete(System.IntPtr userToken) + 0x98 bytes 
    System.dll!System.Net.LazyAsyncResult.ProtectedInvokeCallback(object result, System.IntPtr userToken) + 0xcc bytes  
    System.dll!System.Net.Sockets.BaseOverlappedAsyncResult.CompletionPortCallback(uint errorCode, uint numBytes, System.Threading.NativeOverlapped* nativeOverlapped) + 0x13a bytes    
    mscorlib.dll!System.Threading._IOCompletionCallback.PerformIOCompletionCallback(uint errorCode, uint numBytes, System.Threading.NativeOverlapped* pOVERLAP) + 0x7d bytes    
    [Native to Managed Transition]

[ForNeVeR]

Alright, this is one thread. Could you please show stack for other thread (the one holding the lock)?

[KotM]

I was not aware that second stack trace being useful taking into account that it was stopped on our lock, but I forgot about preceding code... So, please find it below, too:

    [In a sleep, wait, or join] 

    [our code].Utilities.Jabber.PracticeAreaChannel.RespondToQuery(JabberNet.jabber.protocol.client.IQ response = Cannot evaluate expression because the current thread is in a sleep, wait, or join) Line 912 + 0x19 bytes C#

    [our stack here, removed by security reasons]

    JabberNet.dll!JabberNet.jabber.client.JabberClient.FireOnIQ(object sender, JabberNet.jabber.protocol.client.IQ iq = Cannot evaluate expression because the current thread is in a sleep, wait, or join) + 0x2f bytes    
    JabberNet.dll!JabberNet.jabber.connection.XmppStream.JabberNet.jabber.connection.IStanzaEventListener.StanzaReceived(System.Xml.XmlElement elem) + 0x48 bytes   
    JabberNet.dll!JabberNet.jabber.protocol.ElementStream.FireOnElement(System.Xml.XmlElement elem) + 0x2b bytes    
    JabberNet.dll!JabberNet.jabber.protocol.AsynchElementStream.EndTag(byte[] buf, int offset, JabberNet.xpnet.ContentToken ct, JabberNet.xpnet.TOK tok) + 0x134 bytes  
    JabberNet.dll!JabberNet.jabber.protocol.AsynchElementStream.Push(byte[] buf = {byte[4096]}, int offset = 0, int length = 1275) + 0x19c bytes    
    JabberNet.dll!JabberNet.jabber.connection.SocketStanzaStream.JabberNet.bedrock.net.ISocketEventListener.OnRead(JabberNet.bedrock.net.BaseSocket sock = {JabberNet.bedrock.net.AsyncSocket}, byte[] buf, int offset, int length) + 0xbc bytes    
    JabberNet.dll!JabberNet.bedrock.net.AsyncSocket.GotData(System.IAsyncResult ar) + 0x9e bytes    
    System.dll!System.Net.LazyAsyncResult.Complete(System.IntPtr userToken) + 0x52 bytes    
    mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) + 0x15e bytes 
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx) + 0x17 bytes  
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) + 0x52 bytes    
    System.dll!System.Net.ContextAwareResult.Complete(System.IntPtr userToken) + 0x98 bytes 
    System.dll!System.Net.LazyAsyncResult.ProtectedInvokeCallback(object result, System.IntPtr userToken) + 0xcc bytes  
    System.dll!System.Net.Sockets.BaseOverlappedAsyncResult.CompletionPortCallback(uint errorCode, uint numBytes, System.Threading.NativeOverlapped* nativeOverlapped) + 0x13a bytes    
    mscorlib.dll!System.Threading._IOCompletionCallback.PerformIOCompletionCallback(uint errorCode, uint numBytes, System.Threading.NativeOverlapped* pOVERLAP) + 0x7d bytes    
    [Native to Managed Transition]  

[dsparkplug]

Sorry for the delay - other priorities etc.

Here's a MCVE which sends 10000 test messages. The deadlock will generally occur after a few thousand messages with several threads stuck at AsyncSocket.Write and one stuck at XmppStream.BytesWritten.

Note that this is only appears to happen with AutoPresence = true . Also, I wouldn't normally create a new client and authenticate for each message as in this example, but the authentication seems to involved in causing the deadlock.

XMPP server is OpenFire 4.2.3

using JabberNet.bedrock;
using JabberNet.jabber;
using JabberNet.jabber.client;
using JabberNet.jabber.connection;
using JabberNet.jabber.protocol;
using System;
using System.Threading;
using System.Xml;

namespace JabberTest
{
    public class Program
    {

        public static void Main(string[] args)
        {

            string server = "servername";
            string xmppID = "sparky";
            int port = 5222;
            string password = "#######";

            XmppSender xmppSender = new XmppSender();
            int sent = 0;
            for (var i = 0; i < 10000; i++)
            {

                if (!xmppSender.SendMessage(server, xmppID, port, password))
                {
                    Console.WriteLine($"Failed - sent {sent} messages");
                    return;
                }
                sent++;
            }
            Console.WriteLine($"Success - sent {sent} messages"); ;
        }

    }
    public class XmppSender
    {
        private ManualResetEvent _authenticatedEvent;

        public XmppSender()
        {
            _authenticatedEvent = new ManualResetEvent(false);

        }

        private void client_OnAuthenticate(object sender)
        {
            Console.WriteLine("Authenticated");
            _authenticatedEvent.Set();
        }

        private void client_OnConnect(object sender, StanzaStream stream)
        {
            Console.WriteLine("Connected");
        }
        private void client_OnDisconnect(object sender)
        {
            Console.WriteLine("Disconnected");
            _authenticatedEvent.Reset();
        }

        private void client_OnError(object sender, Exception ex)
        {
            Console.WriteLine($"The following error occurred while publishing an XMPP notification:\n{ex}");
            _authenticatedEvent.Set();
        }

        private void client_OnStreamError(object sender, XmlElement rp)
        {
            string info = rp == null ? "No further details" : rp.OuterXml;
            Console.WriteLine($"The following stream error occurred while publishing an XMPP notification:\n{info}");
            _authenticatedEvent.Set();
        }

        private void client_OnAuthError(object sender, XmlElement elem)
        {
            string info = elem == null ? "No further details" : elem.OuterXml;
            Console.WriteLine($"The following authentication error occurred while publishing an XMPP notification:\n{info}");
            _authenticatedEvent.Set();
        }

        public bool SendMessage(string server, string xmppID, int port, string password)
        {

            var jId = JID.Escape(xmppID, server, "Test");

            JabberClient client = new JabberClient();

            client.AutoReconnect = 3f;
            client.User = jId.User;
            client.Server = jId.Server;
            client.NetworkHost = jId.Server;
            client.Port = port;
            client.Resource = jId.Resource;
            client.Password = password;

            client.AutoPresence = true; //note that a deadlock can occur if this is set to true
            client.AutoRoster = false;
            client.SSL = false;
            client.AutoReconnect = 3;
            client.PlaintextAuth = true;
            client.AutoStartCompression = true;
            client.AutoStartTLS = false;
            client.AutoLogin = true;

            client.OnError += new ExceptionHandler(client_OnError);
            client.OnStreamError += new ProtocolHandler(client_OnStreamError);
            client.OnAuthenticate += new ObjectHandler(client_OnAuthenticate);
            client.OnDisconnect += new ObjectHandler(client_OnDisconnect);
            client.OnAuthError += new ProtocolHandler(client_OnAuthError);
            client.OnConnect += new StanzaStreamHandler(client_OnConnect);

            try
            {
                if (!client.IsAuthenticated)
                {
                    _authenticatedEvent.Reset();

                    if (!_authenticatedEvent.WaitOne(0))
                        client.Connect();

                    if (!_authenticatedEvent.WaitOne(10000))
                        return false;
                }
                if (!client.IsAuthenticated)
                    return false;

                jId.Resource = client.Resource;

                Console.WriteLine($"Sending message"); ;
                client.Message(jId.BareJID, "test message");
            }
            finally
            {
                client.Close();

                client.OnError -= client_OnError;
                client.OnStreamError -= client_OnStreamError;
                client.OnAuthenticate -= client_OnAuthenticate;
                client.OnConnect -= client_OnConnect;
                client.OnDisconnect -= client_OnDisconnect;
                client.OnAuthError -= client_OnAuthError;

            }

            return true;
        }
    }
}

[KotM]

Any progress here?

And how dangerous to use AutoPresence = false?

[ForNeVeR]

Sorry, I was preoccupied with other great deeds, so wasn't able to participate in this case. Honestly I think that you, guys, have better opportunity to fix the issue if you have a repro case.

Although, I will now do my best to research the case myself and fix it. I have some spare time and I'm on it right now.

[ForNeVeR]

@KotM, AutoPresence = false is pretty safe (at least if you don't need this functionality actually), and it seems like it reduces the amount of deadlocks in the code.

I was able to reproduce the issue, although, sometimes it happens even with AutoPresence = false. Looks like to terminate the issue completely we'll need pretty heavy rearchitecture of the JabberClient part of the library :(

[ForNeVeR]

For now, I'm thinking about replacing the JabberClient internals with some actor system such as Akka.NET that should solve the problems (partially, at least). Event dispatching should be asynchronous and without manual locking; that way we'll probably defeat the deadlocks.