zeromq / netmq

A 100% native C# implementation of ZeroMQ for .NET
Other
2.93k stars 744 forks source link

Cannot close an uninitialised Msg. #1054

Open LarsHesselberg opened 1 year ago

LarsHesselberg commented 1 year ago

Environment

NetMQ Version:  4.0.1.6+e86706ffb913825b3bc0e1048...  
Operating System: Win10
.NET Version:     4.7

Expected behaviour

No exception

Actual behaviour


Exception Message: Cannot close an uninitialised Msg.
Exception Stack Trace: 
   at NetMQ.Msg.Close()
   at NetMQ.Core.Transports.EncoderBase.Encode(ByteArraySegment& data, Int32 size)
   at NetMQ.Core.Transports.StreamEngine.BeginSending()
   at NetMQ.Core.Transports.StreamEngine.Handle(Action action, SocketError socketError, Int32 bytesTransferred)
   at NetMQ.Core.Transports.StreamEngine.FeedAction(Action action, SocketError socketError, Int32 bytesTransferred)
   at NetMQ.Core.ZObject.ProcessCommand(Command cmd)
   at NetMQ.Core.IOThread.Ready()
   at NetMQ.Core.Utils.Proactor.Loop()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

Steps to reproduce the behaviour

Appears randomly - mostly under heavy load.

Examining the NetMQ code it looks like the issue occurs in EncoderBase.cs Encode function here:

                // If there are no more data to return, run the state machine.
                // If there are still no data, return what we already have
                // in the buffer.
                if (m_toWrite == 0)
                {
                    if (m_newMsgFlag) {
                        m_inProgress.Close();  ** <-- Here: m_inProgress.MsgType maybe  MsgType.Uninitialised. Put in if (m_inProgress.IsInitialised) **
                        m_inProgress.InitEmpty();
                        m_hasMessage = false;
                        break;
                    }

                    Next();
                }

I point at this place because following the call-tree it is the only place I can find where Msg could be Uninitialised.

drewnoakes commented 1 year ago

Are you able to try your proposed fix to see if it stops the issue you are seeing?

LarsHesselberg commented 1 year ago

I have no possibility to reproduce the problem consistently. It happens randomly. But examination of the code reveals that Close() will throw an exception if MsgType is Uninitialised. In my opinion it must be legal to call Close() at any time and repeatedly without getting an exception.

In fact I think the check should be removed in Close() and moved to the XXXInit() functions so these throws exception if not MsgType is Uninitialised, that would be more logical. But I have no deep knowledge to the code complex to see if that will introduced other dysfunctionality/errors.

hryhorasz commented 10 months ago

I run into the same issue randomly too. Difficult to reproduce but looks like it happens under heavy load.

LarsHesselberg commented 1 month ago

We are seeing the problem again. I still think it is better to allow Close() handle !Initialized: Proposed change in Msg.cs:

    /// <summary>
    /// Clear the <see cref="Data"/> and set the MsgType to Invalid.
    /// If this is not a shared-data Msg (MsgFlags.Shared is not set), or it is shared but the reference-counter has dropped to zero,
    /// then return the data back to the BufferPool.
    /// </summary>
    /// <exception cref="FaultException">The object is not initialised.</exception>
    public void Close()
    {
        if (IsInitialised)
        {
            if (MsgType == MsgType.Pool)
            {
                Assumes.NotNull(m_refCount);
                Assumes.NotNull(m_data);

                // if not shared or reference counter drop to zero
                if (!IsShared || m_refCount.Decrement() == 0)
                    BufferPool.Return(m_data);

                EnsureAtomicCounterNull();
            }
        }

        // Uninitialise the frame
        m_data = null;
        MsgType = MsgType.Uninitialised;
    }