Skip to content

Commit 8971822

Browse files
committed
Change the Linux cancel implementation to match MacOS'
cancel on linux was a "real" cancel, but the implementation was unsafe. It took whatever `id` it was given and @ptrfromint'd it. This is problematic since the `id` is user-supplied with virtually no validation. Using the existing MacOS canceled lookup seems both easier and safer than trying to validate the cancellation id.
1 parent 27d1f79 commit 8971822

File tree

1 file changed

+9
-75
lines changed

1 file changed

+9
-75
lines changed

src/runtime/loop.zig

Lines changed: 9 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,10 @@ pub const Loop = struct {
5656
// This is a weak way to cancel all future Zig callbacks.
5757
zig_ctx_id: u32 = 0,
5858

59-
// The MacOS event loop doesn't support cancellation. We use this to track
60-
// cancellation ids and, on the timeout callback, we can can check here
61-
// to see if it's been cancelled.
59+
// We use this to track cancellation ids and, on the timeout callback,
60+
// we can can check here to see if it's been cancelled.
6261
cancelled: std.AutoHashMapUnmanaged(usize, void),
6362

64-
cancel_pool: MemoryPool(ContextCancel),
6563
timeout_pool: MemoryPool(ContextTimeout),
6664
event_callback_pool: MemoryPool(EventCallbackContext),
6765

@@ -79,7 +77,6 @@ pub const Loop = struct {
7977
.io = try IO.init(32, 0),
8078
.js_events_nb = 0,
8179
.zig_events_nb = 0,
82-
.cancel_pool = MemoryPool(ContextCancel).init(alloc),
8380
.timeout_pool = MemoryPool(ContextTimeout).init(alloc),
8481
.event_callback_pool = MemoryPool(EventCallbackContext).init(alloc),
8582
};
@@ -104,7 +101,6 @@ pub const Loop = struct {
104101
self.io.cancel_all();
105102
}
106103
self.io.deinit();
107-
self.cancel_pool.deinit();
108104
self.timeout_pool.deinit();
109105
self.event_callback_pool.deinit();
110106
self.cancelled.deinit(self.alloc);
@@ -175,10 +171,8 @@ pub const Loop = struct {
175171
loop.alloc.destroy(completion);
176172
}
177173

178-
if (comptime CANCEL_SUPPORTED == false) {
179-
if (loop.cancelled.remove(@intFromPtr(completion))) {
180-
return;
181-
}
174+
if (loop.cancelled.remove(@intFromPtr(completion))) {
175+
return;
182176
}
183177

184178
// If the loop's context id has changed, don't call the js callback
@@ -221,73 +215,13 @@ pub const Loop = struct {
221215
return @intFromPtr(completion);
222216
}
223217

224-
const ContextCancel = struct {
225-
loop: *Self,
226-
js_cbk: ?JSCallback,
227-
js_ctx_id: u32,
228-
};
229-
230-
fn cancelCallback(
231-
ctx: *ContextCancel,
232-
completion: *IO.Completion,
233-
result: IO.CancelOneError!void,
234-
) void {
235-
const loop = ctx.loop;
236-
237-
defer {
238-
loop.removeEvent(.js);
239-
loop.cancel_pool.destroy(ctx);
240-
loop.alloc.destroy(completion);
241-
}
242-
243-
// If the loop's context id has changed, don't call the js callback
244-
// function. The callback's memory has already be cleaned and the
245-
// events nb reset.
246-
if (ctx.js_ctx_id != loop.js_ctx_id) return;
247-
248-
// TODO: return the error to the callback
249-
result catch |err| {
250-
switch (err) {
251-
error.NotFound => log.debug("cancel callback: {any}", .{err}),
252-
else => log.err("cancel callback: {any}", .{err}),
253-
}
254-
return;
255-
};
256-
257-
// js callback
258-
if (ctx.js_cbk) |*js_cbk| {
259-
js_cbk.call(null) catch {
260-
loop.cbk_error = true;
261-
};
262-
}
263-
}
264-
265218
pub fn cancel(self: *Self, id: usize, js_cbk: ?JSCallback) !void {
266-
const alloc = self.alloc;
267-
if (comptime CANCEL_SUPPORTED == false) {
268-
try self.cancelled.put(alloc, id, {});
269-
if (js_cbk) |cbk| {
270-
cbk.call(null) catch {
271-
self.cbk_error = true;
272-
};
273-
}
274-
return;
219+
try self.cancelled.put(self.alloc, id, {});
220+
if (js_cbk) |cbk| {
221+
cbk.call(null) catch {
222+
self.cbk_error = true;
223+
};
275224
}
276-
const comp_cancel: *IO.Completion = @ptrFromInt(id);
277-
278-
const completion = try alloc.create(Completion);
279-
errdefer alloc.destroy(completion);
280-
completion.* = undefined;
281-
282-
const ctx = self.alloc.create(ContextCancel) catch unreachable;
283-
ctx.* = ContextCancel{
284-
.loop = self,
285-
.js_cbk = js_cbk,
286-
.js_ctx_id = self.js_ctx_id,
287-
};
288-
289-
self.addEvent(.js);
290-
self.io.cancel_one(*ContextCancel, ctx, cancelCallback, completion, comp_cancel);
291225
}
292226

293227
// Reset all existing JS callbacks.

0 commit comments

Comments
 (0)