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

Cannot close an uninitialised Msg. #1054

Open
LarsHesselberg opened this issue Apr 20, 2023 · 4 comments
Open

Cannot close an uninitialised Msg. #1054

LarsHesselberg opened this issue Apr 20, 2023 · 4 comments

Comments

@LarsHesselberg
Copy link

LarsHesselberg commented Apr 20, 2023

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
Copy link
Member

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

@LarsHesselberg
Copy link
Author

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
Copy link

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

@LarsHesselberg
Copy link
Author

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


/// Clear the 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.
///

/// The object is not initialised.
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;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants