Remove panics from weave runtime to avoid undefined behavior in FFI #139

New issue

Closed

opened 2025-12-30 15:24:23 +00:00 by navicore · 1 comment

navicore commented

2025-12-30 15:24:23 +00:00

(Migrated from github.com)

Background

PR #138 introduced weave/generator support. The implementation includes several panic! calls in extern "C" functions, which can cause undefined behavior when panicking across FFI boundaries.

Current State

In crates/runtime/src/weave.rs, the patch_seq_yield function has two panics:

Line ~382 (yield channel send fails):

if yield_chan.sender.send(value_to_send).is_err() {
    panic!("yield: yield channel closed unexpectedly");
}

Line ~389 (resume channel recv fails):

Err(_) => panic!("yield: resume channel closed unexpectedly"),

Problem

Panicking in extern "C" functions crosses the FFI boundary, which is undefined behavior in Rust. While these error conditions are unlikely in practice (channels are held by the caller), defensive code should avoid UB.

Proposed Solution

Replace panics with the same blocking pattern used for cancellation:

if yield_chan.sender.send(value_to_send).is_err() {
    // Channel unexpectedly closed - block this coroutine forever
    // (it's already in an undefined state, but we avoid UB)
    crate::arena::arena_reset();
    cleanup_strand();
    let (_, rx): (mpmc::Sender<()>, mpmc::Receiver<()>) = mpmc::channel();
    let _ = rx.recv();
    unreachable!();
}

This ensures we never panic across FFI, maintaining well-defined behavior.

Acceptance Criteria

No panic! calls in patch_seq_yield
No panic! calls in patch_seq_resume (verify current state)
Document that unexpected channel closure blocks the coroutine
Tests continue to pass

PR #138 (weave implementation)
Issue #131 (original generator request)

## Background PR #138 introduced weave/generator support. The implementation includes several `panic!` calls in `extern "C"` functions, which can cause undefined behavior when panicking across FFI boundaries. ## Current State In `crates/runtime/src/weave.rs`, the `patch_seq_yield` function has two panics: **Line ~382** (yield channel send fails): ```rust if yield_chan.sender.send(value_to_send).is_err() { panic!("yield: yield channel closed unexpectedly"); } ``` **Line ~389** (resume channel recv fails): ```rust Err(_) => panic!("yield: resume channel closed unexpectedly"), ``` ## Problem Panicking in `extern "C"` functions crosses the FFI boundary, which is undefined behavior in Rust. While these error conditions are unlikely in practice (channels are held by the caller), defensive code should avoid UB. ## Proposed Solution Replace panics with the same blocking pattern used for cancellation: ```rust if yield_chan.sender.send(value_to_send).is_err() { // Channel unexpectedly closed - block this coroutine forever // (it's already in an undefined state, but we avoid UB) crate::arena::arena_reset(); cleanup_strand(); let (_, rx): (mpmc::Sender<()>, mpmc::Receiver<()>) = mpmc::channel(); let _ = rx.recv(); unreachable!(); } ``` This ensures we never panic across FFI, maintaining well-defined behavior. ## Acceptance Criteria - [ ] No `panic!` calls in `patch_seq_yield` - [ ] No `panic!` calls in `patch_seq_resume` (verify current state) - [ ] Document that unexpected channel closure blocks the coroutine - [ ] Tests continue to pass ## Related - PR #138 (weave implementation) - Issue #131 (original generator request)

navicore commented

2025-12-30 16:44:03 +00:00

(Migrated from github.com)

https://github.com/navicore/patch-seq/pull/144