Skip to content

Leak fixes #3

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.

Sign up for GitHub

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

Merged
AlexJReid merged 2 commits into from
May 8, 2026
Merged

Leak fixes #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
619a885
memory leaks
AlexJReid May 8, 2026
111fe9f
rtd lifecycle nit
AlexJReid May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/async_cache.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
const std = @import("std");
const xl_imports = @import("xl_imports.zig");
const xl = xl_imports.xl;
const xl_helpers = @import("xl_helpers.zig");

const allocator = std.heap.c_allocator;

Expand Down Expand Up @@ -57,14 +58,21 @@ pub const AsyncCache = struct {

// If key already exists, just update the value
if (self.map.getEntry(key)) |entry| {
if (entry.value_ptr.xloper != result.xloper) {
xl_helpers.destroyDllOwnedXloper(allocator, entry.value_ptr.xloper);
}
entry.value_ptr.* = result;
return;
}

// New key — dupe it so the cache owns the string
const owned_key = allocator.dupe(u8, key) catch return;
const owned_key = allocator.dupe(u8, key) catch {
xl_helpers.destroyDllOwnedXloper(allocator, result.xloper);
return;
};
self.map.put(owned_key, result) catch {
allocator.free(owned_key);
xl_helpers.destroyDllOwnedXloper(allocator, result.xloper);
};
}

Expand All @@ -74,6 +82,7 @@ pub const AsyncCache = struct {
defer self.unlock();
var it = self.map.iterator();
while (it.next()) |entry| {
xl_helpers.destroyDllOwnedXloper(allocator, entry.value_ptr.xloper);
allocator.free(@constCast(entry.key_ptr.*));
}
self.map.clearAndFree();
Expand Down
13 changes: 0 additions & 13 deletions src/async_cache_tests.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ fn makeNumericXloper(val: f64) *xl.XLOPER12 {
return ptr;
}

fn freeXloper(ptr: *xl.XLOPER12) void {
allocator.destroy(ptr);
}

test "cache miss returns null" {
var cache = async_cache.AsyncCache.init();
defer cache.clear();
Expand All @@ -30,7 +26,6 @@ test "put and get" {
defer cache.clear();

const xloper = makeNumericXloper(42.0);
defer freeXloper(xloper);

cache.put("key1", .{ .xloper = xloper, .completed = true });

Expand All @@ -45,9 +40,7 @@ test "put overwrites existing key" {
defer cache.clear();

const xloper1 = makeNumericXloper(1.0);
defer freeXloper(xloper1);
const xloper2 = makeNumericXloper(2.0);
defer freeXloper(xloper2);

cache.put("key", .{ .xloper = xloper1, .completed = false });
cache.put("key", .{ .xloper = xloper2, .completed = true });
Expand All @@ -63,7 +56,6 @@ test "contains" {
defer cache.clear();

const xloper = makeNumericXloper(0.0);
defer freeXloper(xloper);

try std.testing.expect(!cache.contains("key"));
cache.put("key", .{ .xloper = xloper, .completed = false });
Expand All @@ -74,9 +66,7 @@ test "clear removes all entries" {
var cache = async_cache.AsyncCache.init();

const xloper1 = makeNumericXloper(1.0);
defer freeXloper(xloper1);
const xloper2 = makeNumericXloper(2.0);
defer freeXloper(xloper2);

cache.put("a", .{ .xloper = xloper1, .completed = true });
cache.put("b", .{ .xloper = xloper2, .completed = true });
Expand All @@ -92,9 +82,7 @@ test "in-progress then completed" {
defer cache.clear();

const pending = makeNumericXloper(0.0);
defer freeXloper(pending);
const done = makeNumericXloper(99.0);
defer freeXloper(done);

// Mark in-progress
cache.put("calc|5", .{ .xloper = pending, .completed = false });
Expand All @@ -113,7 +101,6 @@ test "concurrent reads and writes" {
defer cache.clear();

const xloper = makeNumericXloper(42.0);
defer freeXloper(xloper);
cache.put("shared", .{ .xloper = xloper, .completed = true });

const Reader = struct {
Expand Down
4 changes: 2 additions & 2 deletions src/async_infra.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,14 +315,14 @@ fn asyncValueToXloper(val: AsyncValue) *xl.XLOPER12 {
};
},
.string => |v| {
const xv = XLValue.fromUtf8String(allocator, v) catch {
var xv = XLValue.fromUtf8String(allocator, v) catch {
ptr.* = .{
.xltype = xl.xltypeErr | xl.xlbitDLLFree,
.val = .{ .err = xl.xlerrValue },
};
return ptr;
};
ptr.* = xv.m_val;
ptr.* = xv.intoXLOPER12();
ptr.xltype |= xl.xlbitDLLFree;
},
}
Expand Down
8 changes: 4 additions & 4 deletions src/excel_function.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,17 +625,17 @@ pub fn ExcelFunction(comptime meta: anytype) type {
});
} else if (T == []const u8 or T == []u8) {
defer allocator.free(result);
const val = XLValue.fromUtf8String(allocator, result) catch return makeErrorValue();
return heapXloper(val.m_val);
var val = XLValue.fromUtf8String(allocator, result) catch return makeErrorValue();
return heapXloper(val.intoXLOPER12());
} else if (T == [][]const f64 or T == [][]f64) {
defer {
for (result) |row| {
allocator.free(row);
}
allocator.free(result);
}
const val = XLValue.fromMatrix(allocator, result) catch return makeErrorValue();
return heapXloper(val.m_val);
var val = XLValue.fromMatrix(allocator, result) catch return makeErrorValue();
return heapXloper(val.intoXLOPER12());
} else if (T == *xl.XLOPER12) {
return result;
} else {
Expand Down
9 changes: 1 addition & 8 deletions src/framework_entry.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,7 @@ pub fn xlAutoFree12(pxFree: ?*xl.XLOPER12) callconv(.c) void {
// Only free if xlbitDLLFree is set (means we allocated it)

if ((oper.xltype & xl.xlbitDLLFree) != 0) {
// Free string data if present
if ((oper.xltype & xl.xltypeStr) != 0) {
if (oper.val.str) |str_ptr| {
const len = @as(usize, @intCast(str_ptr[0]));
// Free: length prefix (1) + string chars (len) + null terminator (1)
excel_allocator.free(str_ptr[0 .. len + 2]);
}
}
xl_helpers.freeDllOwnedPayload(excel_allocator, oper);
// Free the XLOPER12 structure itself
excel_allocator.destroy(oper);
}
Expand Down
21 changes: 14 additions & 7 deletions src/lua.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,6 @@ fn unlockSharedStore() void {
shared_store_mutex.unlock(io);
}

fn sharedGet(key: []const u8) ?SharedValue {
lockSharedStore();
defer unlockSharedStore();
return shared_store.get(key);
}

fn sharedSet(key: []const u8, value: ?SharedValue) void {
lockSharedStore();
defer unlockSharedStore();
Expand Down Expand Up @@ -202,7 +196,10 @@ fn luaXllGet(L: ?*c.lua_State) callconv(.c) c_int {
};
const key = ptr[0..len];

if (sharedGet(key)) |val| {
lockSharedStore();
defer unlockSharedStore();

if (shared_store.get(key)) |val| {
switch (val) {
.number => |n| c.lua_pushnumber(state, n),
.boolean => |b| c.lua_pushboolean(state, if (b) 1 else 0),
Expand Down Expand Up @@ -271,6 +268,16 @@ pub fn getPoolSize() usize {
pub fn init() !void {
if (pool_initialized) return;

errdefer {
for (&state_pool) |*slot| {
if (slot.L) |L| {
c.lua_close(L);
slot.L = null;
}
slot.in_use = std.atomic.Value(bool).init(false);
}
}

for (&state_pool) |*slot| {
slot.L = createState() catch return error.LuaInitFailed;
}
Expand Down
5 changes: 3 additions & 2 deletions src/lua_function.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ pub fn LuaFunction(comptime meta: anytype) type {
return makeErrorValue();
}
const str = ptr.?[0..len];
const xlval = XLValue.fromUtf8String(allocator, str) catch {
var xlval = XLValue.fromUtf8String(allocator, str) catch {
lua.lua_pop(L, 1);
return makeErrorValue();
};
lua.lua_pop(L, 1);
return heapXloper(xlval.m_val);
return heapXloper(xlval.intoXLOPER12());
},
lua.LUA_TBOOLEAN => {
const b = lua.lua_toboolean(L, -1);
Expand Down Expand Up @@ -381,6 +381,7 @@ pub fn LuaFunction(comptime meta: anytype) type {
freeXloperCopies(&pack.xloper_copies);
allocator.free(pack.key);
allocator.destroy(pack);
async_infra.storeResult(key, makeErrorValue());
}

return rtd_result;
Expand Down
25 changes: 18 additions & 7 deletions src/rtd.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,8 @@ pub fn RtdServer(comptime Handler: type, comptime config: RtdConfig) type {

// ---- server state (non-extern, allocated on heap) ----
const ServerState = struct {
ref_count: ULONG = 1,
ref_count: ULONG = 0,
terminated: bool = false,
ctx: RtdContext = .{},
handler: Handler = .{},
};
Expand Down Expand Up @@ -494,10 +495,7 @@ pub fn RtdServer(comptime Handler: type, comptime config: RtdConfig) type {
s.ref_count -= 1;
const rc = s.ref_count;
if (rc == 0) {
s.handler.onTerminate(&s.ctx);
if (s.ctx.update_event) |evt| {
_ = evt.vtable.Release(@ptrCast(evt));
}
terminateOnce(s);
s.ctx.deinit();
std.heap.c_allocator.destroy(s);
std.heap.c_allocator.destroy(obj);
Expand Down Expand Up @@ -752,13 +750,20 @@ pub fn RtdServer(comptime Handler: type, comptime config: RtdConfig) type {
debugLog("ServerTerminate", .{});
const s = getObj(self_opaque).getState();

terminateOnce(s);
return S_OK;
}

fn terminateOnce(s: *ServerState) void {
if (s.terminated) return;
s.terminated = true;

s.handler.onTerminate(&s.ctx);

if (s.ctx.update_event) |evt| {
_ = evt.vtable.Release(@ptrCast(evt));
s.ctx.update_event = null;
}
return S_OK;
}

fn connectDataStub(_: *anyopaque, _: LONG, _: **SAFEARRAY, _: *c_short, _: *VARIANT) callconv(.winapi) HRESULT {
Expand Down Expand Up @@ -813,7 +818,13 @@ pub fn RtdServer(comptime Handler: type, comptime config: RtdConfig) type {
obj.* = .{ .state = @ptrCast(s) };
_ = @atomicRmw(i32, &g_object_count, .Add, 1, .monotonic);

return queryInterface(@ptrCast(obj), riid, ppv);
const hr = queryInterface(@ptrCast(obj), riid, ppv);
if (hr != S_OK) {
std.heap.c_allocator.destroy(s);
std.heap.c_allocator.destroy(obj);
_ = @atomicRmw(i32, &g_object_count, .Sub, 1, .monotonic);
}
return hr;
}

fn cfLockServer(_: *anyopaque, fLock: c_int) callconv(.winapi) HRESULT {
Expand Down
31 changes: 31 additions & 0 deletions src/xl_helpers.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,34 @@ pub fn debugLogRuntime(msg: []const u8) void {
pub inline fn xlFree(oper: *xl.XLOPER12) void {
_ = xl.Excel12f(xl.xlFree, null, 1, oper);
}

fn xloperBaseType(xltype: @TypeOf(@as(xl.XLOPER12, undefined).xltype)) @TypeOf(@as(xl.XLOPER12, undefined).xltype) {
return xltype & 0xFFF;
}

pub fn freeDllOwnedPayload(allocator: std.mem.Allocator, oper: *xl.XLOPER12) void {
switch (xloperBaseType(oper.xltype)) {
xl.xltypeStr => {
if (oper.val.str) |str_ptr| {
const len = @as(usize, @intCast(str_ptr[0]));
allocator.free(str_ptr[0 .. len + 2]);
}
},
xl.xltypeMulti => {
const rows = @as(usize, @intCast(oper.val.array.rows));
const cols = @as(usize, @intCast(oper.val.array.columns));
const cells = oper.val.array.lparray[0 .. rows * cols];
for (cells) |*cell| {
freeDllOwnedPayload(allocator, cell);
}
allocator.free(cells);
},
else => {},
}
}

pub fn destroyDllOwnedXloper(allocator: std.mem.Allocator, oper: *xl.XLOPER12) void {
if ((oper.xltype & xl.xlbitDLLFree) == 0) return;
freeDllOwnedPayload(allocator, oper);
allocator.destroy(oper);
}
Loading
Loading