Skip to content

fix: oneway shm calls, mem-ring unstuck notify and misc review fixes - #199

Open
lirenjie95 wants to merge 33 commits into
masterfrom
fix/review-fixes
Open

lirenjie95 wants to merge 33 commits into
masterfrom
fix/review-fixes

Conversation

@lirenjie95

@lirenjie95 lirenjie95 commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator
  • r2g: emit a real Go handler for no-return #[mem]/#[shm] functions instead of an ack-only stub that silently discarded every request; box the parameters like the async path and free them on the DROP ack so Go never reads freed request memory, with the unsafe contract for reference parameters documented
  • mem-ring: the Go reader now clears the stuck flag and notifies the unstuck fd on pop so writers parked on a full ring flush their pending items (mirrors the Rust ReadQueue::pop); close the stuck-handshake lost-wakeup window by redelivering directly after mark_stuck; exit the Rust handlers when the peer closes the notification socket instead of busy-spinning; reject zero-sized rings; drop items still buffered on queue drop and wake parked push_with_awaiter futures when the write queue is dropped
  • asmcall: reserve the Win64 32-byte shadow space around both call paths in amd64.s; document that the in-place P-variants do not normalize the stack pointer on amd64 (arm64 does), so their callees must tolerate 8-byte alignment (EN and ZH READMEs)
  • g2r: replace the dyn Any round-trip for response storage with typed c_{trait}_{fn}_drop entries per function so the payload destructors run and the allocation is freed with the correct layout; keep user parameter objects alive across the FFI call; sync the committed generated Go files
  • future: require Exec: Send/Sync in the manual Send/Sync impls of ResponseFuture
  • macro/codegen robustness: reject queue_size = 0 and malformed or unknown #[rust2go::r2g] attributes with spanned errors; reject cyclic struct definitions, u128/i128 and char with clear diagnostics; validate parameter names against Go keywords, raw identifiers and a flat, deliberately conservative list of the identifiers the generated bindings reference, plus per-function conversion-local name pairs; support zero-parameter trait functions (ToRef/FromRef for the unit type); pass the sync-ret slot as a mutable pointer
  • generator: import runtime, cgocall and asmcall only when the generated code actually references them
  • docs: document U+FFFD replacement for non-UTF-8 Go strings, the mem-ring drop quiescence contract, and the asmcall alignment contract
  • tests: exercise oneway mem calls end to end in the test crate and assert delivery through a Go-side invocation counter, plus regression tests for the mem-ring fixes and the new diagnostics

- r2g: emit a real Go handler for no-return #[mem]/#[shm] functions
  instead of an ack-only stub that silently dropped every request
- mem-ring: the Go reader now clears the stuck flag and notifies the
  unstuck fd on pop, so a writer parked on a full ring flushes its
  pending items (mirrors the Rust ReadQueue::pop behavior)
- asmcall: reserve the Win64 32-byte shadow space around calls in
  amd64.s so callee arg spills cannot clobber saved registers
- macro: reject queue_size = 0 and report malformed or unknown
  #[rust2go::r2g] attributes with spanned errors instead of silently
  falling back to defaults
- g2r: keep user parameter objects alive across the FFI call and sync
  the committed generated Go files
- mem-ring: drop items still buffered when a queue is dropped and wake
  parked push_with_awaiter futures when the write queue is dropped
- docs: document U+FFFD replacement for non-UTF-8 Go strings and drop
  char from the supported-type list; clearer codegen error for char
Keep the unsupported-type panic within rustfmt width at all four call
sites and derive Debug for the DropCounter used in the new mem-ring
drop test (Result::unwrap needs it).
@codecov-commenter

codecov-commenter commented Sep 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.81670% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.35%. Comparing base (4ba6b18) to head (7baee66).

Files with missing lines Patch % Lines
mem-ring/src/queue.rs 64.64% 35 Missing ⚠️
rust2go-macro/src/lib.rs 92.18% 5 Missing ⚠️
rust2go-common/src/r2g/emit_go.rs 86.66% 4 Missing ⚠️
rust2go-common/src/g2r/mod.rs 97.14% 2 Missing ⚠️
mem-ring/src/eventfd.rs 80.00% 1 Missing ⚠️
rust2go-common/src/g2r/emit_c.rs 88.88% 1 Missing ⚠️
rust2go-common/src/r2g/emit_rust.rs 83.33% 1 Missing ⚠️
rust2go-convert/src/convert.rs 94.11% 1 Missing ⚠️

❌ Your project status has failed because the head coverage (96.35%) is below the target coverage (97.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #199      +/-   ##
==========================================
- Coverage   97.02%   96.35%   -0.67%     
==========================================
  Files          24       23       -1     
  Lines        4098     4530     +432     
==========================================
+ Hits         3976     4365     +389     
- Misses        122      165      +43     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

If a writer was descheduled between the failed push and mark_stuck, a
reader could drain the whole ring and go to sleep without ever seeing
the stuck flag, leaving the parked item and the flusher asleep on both
fds. Re-check the ring after marking stuck on both the Rust and Go push
entry points and deliver directly when space has appeared, mirroring
the re-check the background flushers already do.
…ings

The Rust working/unstuck handlers discarded the notification read
result, so once the peer closed its socket they woke immediately and
forever (a hot spin); the Go side already exits in this case. Awaiter
now reports the closed peer and both handlers exit, with the unstuck
handler waking parked push_with_awaiter futures whose items can no
longer be delivered. Queue::new(0) and Go NewQueue with BufferLen 0
produced permanently-full rings; reject them up front.
- r2g: box oneway #[mem]/#[shm] parameters into the slab like the async
  path does and free them when the DROP ack arrives, so Go never reads
  freed request memory; document the unsafe contract for reference
  parameters
- g2r: replace the dyn Any round-trip for response storage with a typed
  c_{trait}_{fn}_drop entry per function, so the payload destructors run
  and the allocation is freed with the correct layout
- future: require Exec: Send/Sync in the manual Send/Sync impls of
  ResponseFuture instead of asserting them without bounds
- asmcall: normalize the stack alignment in the CallFuncP* trampolines
  (Go only guarantees 8-byte alignment; SysV/Win64 want 16 at the call)
- codegen robustness: reject cyclic struct definitions, reject u128/i128
  with a clear error, reject mem parameters whose names collide with the
  generated ring-handler locals, and support zero-parameter trait
  functions (ToRef/FromRef for the unit type)
- r2g sync-return slot is now passed as *mut derived from &mut
- sync the committed generated Go files with the new emitters
@lirenjie95
lirenjie95 requested a review from ihciah September 24, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants