ci: add code coverage, add core crate tests, and fix mem-ring fd/heap safety issues

[lirenjie95]

Summary

This PR adds code coverage collection to CI, significantly expands the test suite of core crates, and fixes two real memory/fd safety issues in mem-ring that were exposed by the new tests.

Current coverage (Codecov, this branch): 82.6% lines (2873 / 3478, 605 missed) across 18 files. Branch coverage is not tracked yet (cargo-llvm-cov lcov without --branch). Previously CI had no coverage measurement at all.

Changes

CI: coverage collection (.github/workflows/ci.yml)

• New coverage job: collects Rust coverage with cargo-llvm-cov (lcov) and Go coverage for test/go (Go tests were never run in CI before), and uploads both to Codecov.
• sccache is disabled for this job (incompatible with coverage instrumentation).
• mem-ring is tested/covered in a separate cargo invocation (-p mem-ring) — see the note below.

New tests (+~650 lines)

• rust2go-convert/src/convert.rs: MemType, ToRef/FromRef roundtrips for primitives/String/Vec/Option/nested Vec/tuples, CopyStruct buffer layout, Writer.
• rust2go/src/slot.rs: atomic slot read/write, attachments, waker wakeup, ptr roundtrip, drop orders.
• rust2go/src/future.rs: ResponseFuture poll lifecycle (sync/async completion, new_without_req, poll-after-ready panic, drop paths).
• mem-ring/src/queue.rs: sync tests for push/pop/wrap-around/new_from_meta/WakerSlot; async tests for pending tasks, notify_manually, push_with_awaiter.
• mem-ring/src/eventfd.rs: socketpair notify, dup.
• test/go/impl_test.go: fixed to use the initialized TestCallImpl — these tests were previously broken (constructed an empty Demo, so all state-dependent cases failed) and never ran in CI.

mem-ring fixes

1. Use-after-free in Awaiter::wait (mem-ring/src/eventfd.rs)
The monoio version read into a thread_local buffer via a raw pointer (RawBuf). A spawned unstuck_handler task can leak at runtime drop (detached JoinHandle + waker/Op Rc cycle), leaving an in-flight kernel Recv holding a dangling pointer to freed TLS memory. A later notify() completes that read and the kernel writes into reused heap. Now an owned Vec is passed to the read op so the buffer lives as long as the op itself.

2. fd double close (mem-ring/src/queue.rs)
Queue::read, run_handler and Queue::write handed the queue's own fd to the peer-side Notifier/Awaiter, while Queue::drop also closes it — one fd was closed twice. With parallel tests the fd number can be reused in between by an unrelated owner (another test thread's socketpair / epoll / uring fd), which then gets its descriptor closed from under it. The fds are now dup()'ed so each owner closes an independent descriptor.

Testing

• Full workspace: cargo test --all-features --all-targets, cargo clippy --all-features --all-targets -- --deny warnings, cargo fmt --check — all green (also verified locally on macOS + go1.18).
• mem-ring suite run in a loop on CI (uring + legacy drivers): 15/15 passes after the fixes (was ~100% crash before).

Note for reviewers (the heap corruption saga) — @ihciah

The new mem-ring tests initially crashed CI almost 100% with tcache_thread_shutdown(): unaligned tcache chunk detected (glibc heap metadata corruption) — only on Linux, only in the full-suite binary. Investigation (valgrind, ASan, gdb, single-threaded, test-elimination bisect — rr was unavailable on the runner) showed:

1. Every subset of tests passed; only all 10 together crashed. valgrind and ASan reported zero memory errors.
2. The fd double close (fix 2 above) was confirmed as a real contributor and fixing it took the crash rate from ~100% to 0 for the -p mem-ring binary (15/15 loop passes, both drivers).
3. However, the workspace-wide test binary (cargo test --all-features --all-targets) still crashes, while the -p mem-ring build of the same tests never does. The discriminant is monoio's sync feature: workspace feature unification pulls monoio/sync (required by the test crate) into mem-ring's test binary. With sync enabled, the combination of monoio 0.2.4's sync machinery and mem-ring's "spawned task that only exits with the runtime" pattern still corrupts the heap on test teardown (both io_uring and legacy drivers). mem-ring itself does not use monoio/sync, so CI now tests mem-ring with the feature set declared by its own Cargo.toml.
4. Point 3 looks like a genuine monoio issue (possibly around sync-mode unpark/eventfd and runtime teardown with leaked parked tasks) and may deserve a dedicated repro + fix in monoio — I did not root-cause it to a specific write instruction. Happy to open an issue on monoio with the full investigation log if you'd like.

Related Issue

N/A — CI/test infrastructure improvement; no tracking issue.

[codecov-commenter]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[ihciah]

The overall direction looks good. In particular, the fix for the Awaiter::wait buffer lifetime and the additional core tests are valuable. However, there are still several fd ownership issues that should be addressed before merging.

Must fix

1. queue.rs:532-533 should not fall back to the original fd when dup() fails.

If the process hits EMFILE/ENFILE, dup() returns an error. The fallback then gives the same fd to both Notifier and Queue, so the fd is double-closed during destruction. If the descriptor number has been reused, this may close an unrelated fd.

The same pattern appears at queue.rs:559-561, queue.rs:593-595, and queue.rs:620-622.

Please propagate the dup() error or implement an explicit ownership transfer instead of falling back to the original fd.

2. Queue instances created by new_from_meta() leak fds.

new_from_meta() sets do_drop to false, and Queue::drop therefore does not close the original fds. After this change, read()/write() only close the duplicated fds, leaving the original peer fds open indefinitely.

This leaks fds every time new_from_meta().read() or .write() is used and can eventually exhaust the process fd limit. Memory ownership and fd ownership should be tracked separately.

Suggestion

3. dup() drops FD_CLOEXEC.

new_pair() creates sockets with SOCK_CLOEXEC on Linux, but a regular dup() creates a descriptor without the close-on-exec flag. These internal fds may therefore be inherited by child processes.

Please use F_DUPFD_CLOEXEC or another duplication method that preserves close-on-exec semantics.

One additional note: --all-features still only exercises the monoio branch, so the tokio branch is not covered by the new CI tests. This appears to be an existing CI gap, but it would be good to add a separate tokio-feature test.

Local validation passed for the workspace tests, mem-ring tests, Clippy, rustfmt, and Go tests.

[lirenjie95]

Thanks for the careful review! All points are addressed in 8433bd2.

1. dup() fallback → explicit fd ownership transfer

Instead of propagating the dup() error (which would have changed read()'s signature), I went with the explicit ownership transfer option. read()/write()/read_with_tokio_handle()/write_with_tokio_handle()/run_handler() now move the fd into the Notifier/Awaiter via mem::replace(&mut self.xxx_fd, -1), so each descriptor is closed exactly once by its owner. No dup(), no error path, no fallback. If Awaiter::from_raw_fd fails, the fd is still closed by the dropped std UnixStream, so the error path is leak-free as well.

2. new_from_meta() fd leak — memory vs fd ownership separated

do_drop now only controls freeing the shared memory (buffer + head/tail/working/stuck atomics). Queue::drop unconditionally closes the fds it still owns (skipping -1, i.e. transferred ones). Since Queue::new puts the socketpair's peer ends into the meta, the meta receiver is the sole owner of those descriptor numbers, so a new_from_meta queue closing them on drop cannot double-close the creator's fds. The new_from_meta safety docs were updated to state this contract.

Two new tests cover this: drop_closes_fds (both owner and from-meta queues close their fds on drop) and drop_after_read_write_transfers. They detect closure via POLLHUP on the peer end of the socketpair rather than fcntl(F_GETFD) on the closed fd, because parallel test threads can immediately reuse a freed fd number and made the EBADF check flaky.

3. dup() dropping FD_CLOEXEC

Moot now: with all call sites converted to ownership transfer, dup() (and its test) is removed entirely — no descriptor duplication remains, so no close-on-exec semantics can be lost.

4. Tokio branch not exercised in CI

Added a Test mem-ring (tokio) step (cargo test -p mem-ring --no-default-features --features tokio --all-targets) to the build-and-test job. This also exposed a pre-existing bug: mem-ring's tokio branch did not compile standalone because AsyncReadExt requires tokio's io-util feature — fixed in mem-ring/Cargo.toml.

Local validation: mem-ring tests pass under monoio+tpc, monoio (no tpc), and tokio feature sets (repeated runs, no flakes); workspace tests, clippy --all-features --all-targets --deny warnings (plus the mem-ring tokio feature set), cargo fmt --check, and the Go tests are all green.

[ihciah]

Re-review result: the main issues from the previous review appear to be fixed:

• The dup() fallback has been removed.
• FD ownership is now tracked separately from memory ownership.
• The non-owner queue FD leak is fixed.
• The CLOEXEC issue is no longer introduced because dup() was removed.
• A separate Tokio feature test was added to CI.
• Both monoio and Tokio test suites pass.

One remaining concern:

P2: ReadQueue::meta() / WriteQueue::meta() can now return invalid FDs

Queue::read() and Queue::write() set transferred FDs to -1, while ReadQueue::meta() and WriteQueue::meta() still expose QueueMeta.

As a result:

• ReadQueue::meta() returns unstuck_fd = -1;
• WriteQueue::meta() returns both FDs as -1.

If these methods are only intended to be called before read()/write(), please document that restriction. Otherwise, the API should preserve valid metadata or return a Result.

Other than this API concern, the changes look good.

[lirenjie95]

Thanks for the re-review! The P2 concern is addressed in 63bd3fa by documenting the restriction on both methods:

• ReadQueue::meta(): only the memory fields of the returned meta are meaningful; unstuck_fd is -1 because its ownership was transferred to the internal notifier in Queue::read().
• WriteQueue::meta(): both fds are -1 (transferred to the notifier/awaiter in Queue::write()).
• Both docs now point out that a shareable meta for a peer should be obtained from Queue::new.

I chose documenting over returning the live fds deliberately: these two methods have no callers in the repo, and a QueueMeta is only safely shareable when it carries the dedicated peer-end descriptors handed out by Queue::new. Returning the queue's own-end fds (e.g. the notifier's fd) would let a new_from_meta() receiver adopt — and eventually close — descriptors owned by these queues, reintroducing the double-close problem. Reporting -1 makes any such misuse fail loudly with EBADF instead.

The drop_after_read_write_transfers test now also asserts this documented behavior (unstuck_fd == -1, working_fd still valid for ReadQueue).

mem-ring tests pass under both monoio (default + tpc) and tokio feature sets; clippy --deny warnings and rustfmt are clean.

[ihciah]

LGTM! Thanks @lirenjie95 ! It's a very good work.