Skip to content

Commit

Permalink
Fix debug-only crash (#16279)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarred-Sumner authored Jan 9, 2025
1 parent 9bca80c commit 313bf86
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 29 deletions.
4 changes: 2 additions & 2 deletions packages/bun-usockets/src/internal/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ enum {
#define POLL_TYPE_MASK (POLL_TYPE_KIND_MASK | POLL_TYPE_POLLING_MASK)

/* Bun APIs implemented in Zig */
void Bun__lock(uint32_t *lock);
void Bun__unlock(uint32_t *lock);
void Bun__lock(zig_mutex_t *lock);
void Bun__unlock(zig_mutex_t *lock);

struct addrinfo_request;
struct addrinfo_result_entry {
Expand Down
14 changes: 13 additions & 1 deletion packages/bun-usockets/src/internal/loop_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@

#include <stdint.h>

#if defined(__APPLE__)
#include <os/lock.h>
typedef os_unfair_lock zig_mutex_t;
#elif defined(__linux__)
typedef uint32_t zig_mutex_t;
#elif defined(_WIN32)
// SRWLOCK
typedef void* zig_mutex_t;
#else
#error "Unsupported platform"
#endif

// IMPORTANT: When changing this, don't forget to update the zig version in uws.zig as well!
struct us_internal_loop_data_t {
struct us_timer_t *sweep_timer;
Expand All @@ -39,7 +51,7 @@ struct us_internal_loop_data_t {
int low_prio_budget;
struct us_connecting_socket_t *dns_ready_head;
struct us_connecting_socket_t *closed_connecting_head;
uint32_t mutex;
zig_mutex_t mutex;
void *parent_ptr;
char parent_tag;
/* We do not care if this flips or not, it doesn't matter */
Expand Down
32 changes: 12 additions & 20 deletions packages/bun-usockets/src/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,30 @@
#ifndef WIN32
#include <sys/ioctl.h>
#endif
#include "wtf/Platform.h"

#if ASSERT_ENABLED
extern const size_t Bun__lock__size;
extern void __attribute((__noreturn__)) Bun__panic(const char* message, size_t length);
#define BUN_PANIC(message) Bun__panic(message, sizeof(message) - 1)
#endif

/* The loop has 2 fallthrough polls */
void us_internal_loop_data_init(struct us_loop_t *loop, void (*wakeup_cb)(struct us_loop_t *loop),
void (*pre_cb)(struct us_loop_t *loop), void (*post_cb)(struct us_loop_t *loop)) {
// We allocate with calloc, so we only need to initialize the specific fields in use.
loop->data.sweep_timer = us_create_timer(loop, 1, 0);
loop->data.recv_buf = malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2);
loop->data.send_buf = malloc(LIBUS_SEND_BUFFER_LENGTH);
loop->data.ssl_data = 0;
loop->data.head = 0;
loop->data.iterator = 0;
loop->data.closed_udp_head = 0;
loop->data.closed_head = 0;
loop->data.low_prio_head = 0;
loop->data.low_prio_budget = 0;

loop->data.pre_cb = pre_cb;
loop->data.post_cb = post_cb;
loop->data.iteration_nr = 0;

loop->data.closed_connecting_head = 0;
loop->data.dns_ready_head = 0;
loop->data.mutex = 0;

loop->data.parent_ptr = 0;
loop->data.parent_tag = 0;

loop->data.closed_context_head = 0;
loop->data.jsc_vm = 0;

loop->data.wakeup_async = us_internal_create_async(loop, 1, 0);
us_internal_async_set(loop->data.wakeup_async, (void (*)(struct us_internal_async *)) wakeup_cb);
#if ASSERT_ENABLED
if (Bun__lock__size != sizeof(loop->data.mutex)) {
BUN_PANIC("The size of the mutex must match the size of the lock");
}
#endif
}

void us_internal_loop_data_free(struct us_loop_t *loop) {
Expand Down
21 changes: 16 additions & 5 deletions src/Mutex.zig
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,16 @@ const Impl = if (builtin.mode == .Debug and !builtin.single_threaded)
else
ReleaseImpl;

const ReleaseImpl =
pub const ReleaseImpl =
if (builtin.os.tag == .windows)
WindowsImpl
else if (builtin.os.tag.isDarwin())
DarwinImpl
else
FutexImpl;

pub const ExternImpl = ReleaseImpl.Type;

const DebugImpl = struct {
locking_thread: std.atomic.Value(Thread.Id) = std.atomic.Value(Thread.Id).init(0), // 0 means it's not locked.
impl: ReleaseImpl = .{},
Expand Down Expand Up @@ -96,7 +98,7 @@ const DebugImpl = struct {
// SRWLOCK on windows is almost always faster than Futex solution.
// It also implements an efficient Condition with requeue support for us.
const WindowsImpl = struct {
srwlock: windows.SRWLOCK = .{},
srwlock: Type = .{},

fn tryLock(self: *@This()) bool {
return windows.kernel32.TryAcquireSRWLockExclusive(&self.srwlock) != windows.FALSE;
Expand All @@ -111,11 +113,13 @@ const WindowsImpl = struct {
}

const windows = std.os.windows;

pub const Type = windows.SRWLOCK;
};

// os_unfair_lock on darwin supports priority inheritance and is generally faster than Futex solutions.
const DarwinImpl = struct {
oul: c.os_unfair_lock = .{},
oul: Type = .{},

fn tryLock(self: *@This()) bool {
return c.os_unfair_lock_trylock(&self.oul);
Expand All @@ -130,6 +134,7 @@ const DarwinImpl = struct {
}

const c = std.c;
pub const Type = c.os_unfair_lock;
};

const FutexImpl = struct {
Expand Down Expand Up @@ -196,16 +201,22 @@ const FutexImpl = struct {
Futex.wake(&self.state, 1);
}
}

pub const Type = u32;
};

const Mutex = @This();

pub fn spinCycle() void {}

export fn Bun__lock(ptr: *Mutex) void {
// These have to be a size known to C.
export fn Bun__lock(ptr: *ReleaseImpl) void {
ptr.lock();
}

export fn Bun__unlock(ptr: *Mutex) void {
// These have to be a size known to C.
export fn Bun__unlock(ptr: *ReleaseImpl) void {
ptr.unlock();
}

export const Bun__lock__size: usize = @sizeOf(ReleaseImpl);
2 changes: 1 addition & 1 deletion src/deps/uws.zig
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub const InternalLoopData = extern struct {
low_prio_budget: i32,
dns_ready_head: *ConnectingSocket,
closed_connecting_head: *ConnectingSocket,
mutex: u32, // this is actually a bun.Mutex
mutex: bun.Mutex.ReleaseImpl.Type,
parent_ptr: ?*anyopaque,
parent_tag: c_char,
iteration_nr: usize,
Expand Down

0 comments on commit 313bf86

Please sign in to comment.