From 760d8c1cdd870aa3f48aceb6c6a9de0e9b57b001 Mon Sep 17 00:00:00 2001 From: Jack Mordaunt Date: Thu, 12 Jun 2025 15:41:48 -0300 Subject: [PATCH] core/sync/chan.send: return false if channel is closed while blocked This commit makes send behave the same as recv: that the call will return false if the channel is closed while a thread is waiting on the blocking operation. Prior logic would have send return true even if the channel was actually closed rather than read from. Docs adjusted to make this clear. Tests added to lock in this behaviour. --- core/sync/chan/chan.odin | 23 ++++++---- tests/core/sync/chan/test_core_sync_chan.odin | 44 +++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/core/sync/chan/chan.odin b/core/sync/chan/chan.odin index 1d91556b5..1f434f004 100644 --- a/core/sync/chan/chan.odin +++ b/core/sync/chan/chan.odin @@ -420,8 +420,8 @@ as_recv :: #force_inline proc "contextless" (c: $C/Chan($T, $D)) -> (r: Chan(T, Sends the specified message, blocking the current thread if: - the channel is unbuffered - the channel's buffer is full -until the channel is being read from. `send` will return -`false` when attempting to send on an already closed channel. +until the channel is being read from or the channel is closed. `send` will +return `false` when attempting to send on an already closed channel. **Inputs** - `c`: The channel @@ -492,8 +492,9 @@ try_send :: proc "contextless" (c: $C/Chan($T, $D), data: T) -> (ok: bool) where Reads a message from the channel, blocking the current thread if: - the channel is unbuffered - the channel's buffer is empty -until the channel is being written to. `recv` will return -`false` when attempting to receive a message on an already closed channel. +until the channel is being written to or the channel is closed. `recv` will +return `false` when attempting to receive a message on an already closed +channel. **Inputs** - `c`: The channel @@ -566,8 +567,8 @@ try_recv :: proc "contextless" (c: $C/Chan($T)) -> (data: T, ok: bool) where C.D Sends the specified message, blocking the current thread if: - the channel is unbuffered - the channel's buffer is full -until the channel is being read from. `send_raw` will return -`false` when attempting to send on an already closed channel. +until the channel is being read from or the channel is closed. `send_raw` will +return `false` when attempting to send on an already closed channel. Note: The message referenced by `msg_out` must match the size and alignment used when the `Raw_Chan` was created. @@ -633,6 +634,11 @@ send_raw :: proc "contextless" (c: ^Raw_Chan, msg_in: rawptr) -> (ok: bool) { sync.signal(&c.r_cond) } sync.wait(&c.w_cond, &c.mutex) + + if c.closed { + return false + } + ok = true } return @@ -642,8 +648,9 @@ send_raw :: proc "contextless" (c: ^Raw_Chan, msg_in: rawptr) -> (ok: bool) { Reads a message from the channel, blocking the current thread if: - the channel is unbuffered - the channel's buffer is empty -until the channel is being written to. `recv_raw` will return -`false` when attempting to receive a message on an already closed channel. +until the channel is being written to or the channel is closed. `recv_raw` +will return `false` when attempting to receive a message on an already closed +channel. Note: The location pointed to by `msg_out` must match the size and alignment used when the `Raw_Chan` was created. diff --git a/tests/core/sync/chan/test_core_sync_chan.odin b/tests/core/sync/chan/test_core_sync_chan.odin index ae7456d99..8267d6bef 100644 --- a/tests/core/sync/chan/test_core_sync_chan.odin +++ b/tests/core/sync/chan/test_core_sync_chan.odin @@ -228,6 +228,50 @@ test_full_buffered_closed_chan_deadlock :: proc(t: ^testing.T) { testing.expect(t, !chan.send(ch, 32)) } +// Ensures that if a thread is doing a blocking send and the channel +// is closed, it will report false to indicate a failure to complete. +@test +test_fail_blocking_send_on_close :: proc(t: ^testing.T) { + ch, ch_alloc_err := chan.create(chan.Chan(int), context.allocator) + assert(ch_alloc_err == nil, "allocation failed") + defer chan.destroy(ch) + + sender := thread.create_and_start_with_poly_data(ch, proc(ch: chan.Chan(int)) { + assert(!chan.send(ch, 42)) + }) + + for !chan.can_recv(ch) { + thread.yield() + } + + testing.expect(t, chan.close(ch)) + thread.join(sender) + thread.destroy(sender) +} + +// Ensures that if a thread is doing a blocking read and the channel +// is closed, it will report false to indicate a failure to complete. +@test +test_fail_blocking_recv_on_close :: proc(t: ^testing.T) { + ch, ch_alloc_err := chan.create(chan.Chan(int), context.allocator) + assert(ch_alloc_err == nil, "allocation failed") + defer chan.destroy(ch) + + reader := thread.create_and_start_with_poly_data(ch, proc(ch: chan.Chan(int)) { + v, ok := chan.recv(ch) + assert(!ok) + assert(v == 0) + }) + + for !chan.can_send(ch) { + thread.yield() + } + + testing.expect(t, chan.close(ch)) + thread.join(reader) + thread.destroy(reader) +} + // Ensures that try_send for unbuffered channels works as expected. // If 1 reader of a channel, and 3 try_senders, only one of the senders // will succeed and none of them will block.