jeikabu / nng.NETCore

MIT License
40 stars 22 forks source link

"malloc(): memory corruption (fast)" when using ISendSocket.SendMsg(IMessage) #80

Closed biocommando closed 4 years ago

biocommando commented 4 years ago

I encountered an issue where I get "malloc(): memory corruption (fast)" or other similar errors (using .NET Core 3.1 on Linux) when using ISendSocket.SendMsg(IMessage). Here's the minimal program that will reproduce the issue:

using System.Threading.Tasks;

using nng;
using System.IO;

namespace NngBug
{
    class Program
    {
        static void Main(string[] args)
        {
            var path = Path.GetDirectoryName(typeof(Program).Assembly.Location);
            var ctx = new nng.NngLoadContext(path);
            var factory = nng.NngLoadContext.Init(ctx);
            var addr = "tcp://127.0.0.1:5000";
            using (var repSocket = factory.ReplierOpen().ThenListen(addr).Unwrap())
            using (var reqSocket = factory.RequesterOpen().ThenDial(addr).Unwrap())
            {
                Task.Run(() =>
                {
                    while (true)
                    {
                        // the code block causing the issue
                        using (var msg = factory.CreateMessage())
                        {
                            msg.Append(new byte[100]);
                            reqSocket.SendMsg(msg).Unwrap();
                        }
                        // if I use this instead it works without errors:
                        // reqSocket.Send(new byte[100]).Unwrap();
                        using (var resp = reqSocket.RecvMsg().Unwrap())
                        {
                        }
                    }
                });
                while (true)
                {
                    using (var msg = repSocket.RecvMsg().Unwrap())
                    {
                        repSocket.Send(new byte[100]);
                    }
                }
            }
        }
    }
}
jeikabu commented 4 years ago

This with a particular version, or HEAD?

jeikabu commented 4 years ago

It's possibly related to nng_sendmsg semantics where on success NNG takes ownership of the message. But using will mean it is Dispose'd by the caller- which calls free(). The behaviour of ISocket::SendMsg likely needs to be changed to release ownership of IMessage::NngMsg if the calls succeeds.

jeikabu commented 4 years ago

That seems to be it. In Socket.cs change SendMsg to:

public NngResult<Unit> SendMsg(IMessage message, Defines.NngFlag flags = default)
        {
            var res = Unit.OkIfZero(nng_sendmsg(NngSocket, message.NngMsg, flags));
            if (res.IsOk())
            {
                var _ = message.Take();
            }
            return res;
        }

Let me know if that works for you. I'll need to check if this is needed elsewhere.

biocommando commented 4 years ago

I'm using the (then) latest release version through NuGet (1.2.4-rc0).

When I replace this line reqSocket.SendMsg(msg).Unwrap(); with the code below the application doesn't crash anymore.

var r = reqSocket.SendMsg(msg);
if (r.IsOk()) _ = msg.Take();
r.Unwrap();

So yes, your suggested fix does the job :)

jeikabu commented 4 years ago

Great I'll go ahead and apply this fix.

Sorry about the nuget package. My code signing cert expired in January and I finally got a new one.