nine-lives-later / zzmq

Zig Binding for ZeroMQ
Mozilla Public License 2.0
19 stars 2 forks source link

Segmentation fault in ZMessage #8

Open ritalin opened 3 months ago

ritalin commented 3 months ago

For sending, if ZMessage is created in stack frame, segmentation fault was reported.

Following code is example.

fn someFunction(allocator: Allocator, socket: *ZSocket, text: []const u8) !void {
    var msg =try  ZMessage.init(allocator, text);
    defer msg.deinit();

    try socket.send(&msg, .{});
} // <---- ZMessage instance is invalidated here

If ZExternalMessage.refRelease is called after exit function, it will be released dangling pointer.

Resolving this issue, it must allocate ZInternalMessage into heap.

For instance: in ZMessage.init method

const ZMessageImpl = union(ZMessageType) {
    Internal: *ZMessageInternal, // <---- hold as pointer
    External: ZMessageExternal,
};

pub const ZMessage = struct {
    impl_: ZMessageImpl,

    /// Creates a message based on the provided data.
    ///
    /// The data is being copied into the message.
    pub fn init(allocator: std.mem.Allocator, d: []const u8) !ZMessage {
+       const impl = try allocator.create(ZMessageInternal);  // <----- heap allocation
+       impl.* = try ZMessageInternal.init(allocator, d);

        return .{ .impl_ = .{
-            .internal = try ZMessageInternal.init(allocator, d),
+            .Internal = impl,
        } };
    }

    // (snip)
};

const ZMessageInternal = struct {
    // (snip) 

    fn refRelease(self: *ZMessageInternal) void {
        const prev = @atomicRmw(usize, &self.refCounter_, .Sub, 1, .seq_cst);

        if (prev == 1) { // it's now zero
            if (self.allocator_) |a| {
                a.free(self.data_);
+               a.destroy(self);  // < ---- suicide safety 
            }
        }
    }
};

Since this ZMessageInternal variant of ZMessage goes on keeping in heap memory, It can release safety.


In addition to this solution, as ZMessage.initUnmanaged does not always pass allocator, it may be need InternalUnmanaged variant of ZMessageImpl.

fkollmann commented 3 months ago

Thanks for reporting the issue! Please give me some time to reproduce the issue and have a look at possible solutions. Also thanks for proposing one solution here.