Diff Detective

Code Review is the way to
understand code.

Code review that organizes diffs, detects moved code, and flags potential bugs.

Try on any public GitHub PR URL

Or connect this bot to your repository for automatic PR reviews.Connect repository
1Add setTimeout.clock marker for fake timer detection
0/1
FakeTimers.zigsrc/bun.js/test/timers/FakeTimers.zig+22
@@ -188,7 +188,7 @@
pub fn setFakeTimerMarker(this: *FakeTimers) void {
const clock = this.clock;
-- clock.is_fake = false;
++ clock.is_fake = true;
if (clock.setTimeout) |f| {
f.mark();
}
2Fix setTimeout behavior in Node.js compatibility layer
1/3
Timer.zigsrc/bun.js/Timer.zig+45-12
@@ -42,6 +42,19 @@
const Timer = struct {
handle: uv.uv_timer_t,
++ is_clock_marker: bool = false,
++ clock_id: u32 = 0,
callback: JSValue,
};
@@ -98,4 +111,8 @@
fn setTimeout(this: *Timer, ms: u64) void {
-- this.handle.start(ms, 0);
++ if (this.is_clock_marker) {
++ this.markClock();
++ }
++ this.handle.start(ms, 0);
}
clock.zigsrc/bun.js/test/timers/clock.zig+8
@@ -5,5 +5,13 @@
pub const Clock = struct {
is_fake: bool = false,
++ /// Clock marker injected by setTimeout
++ setTimeout: ?*const fn () void = null,
++ setInterval: ?*const fn () void = null,
++ clearTimeout: ?*const fn () void = null,
tick: u64 = 0,
};
3Add regression test for FakeTimers clock detection
0/0
FakeTimers.test.zigsrc/bun.js/test/timers/FakeTimers.test.zig+67
@@ -0,0 +1,67 @@
++const std = @import("std");
++const FakeTimers = @import("FakeTimers.zig");
++
++test "setTimeout.clock marker is set" {
++ var timers = FakeTimers.init();
++ defer timers.deinit();
++
++ timers.install();
++ try std.testing.expect(timers.clock.is_fake == true);
++ try std.testing.expect(timers.clock.setTimeout != null);
++}
++
++test "clearTimeout removes marker" {
++ var timers = FakeTimers.init();
++ defer timers.deinit();
++ timers.install();
++ timers.uninstall();
++ try std.testing.expect(timers.clock.is_fake == false);
++}

Changes in PR

FakeTimers.zig
Timer.zig
clock.zig
FakeTimers.test.zig

Bug Catcher

setFakeTimerMarker sets clock to false instead of true

Bug · FakeTimers.zig:188

Timer.is_clock_marker field may cause alignment issues in packed structs

Investigate · Timer.zig:43

Missing null check before dereferencing setTimeout function pointer

Investigate · Timer.zig:114