Skip to content

Unlock shared writer after close#26

Open
jeet-dekivadia wants to merge 1 commit into
janestreet:masterfrom
jeet-dekivadia:unlock-shared-writer-after-close
Open

Unlock shared writer after close#26
jeet-dekivadia wants to merge 1 commit into
janestreet:masterfrom
jeet-dekivadia:unlock-shared-writer-after-close

Conversation

@jeet-dekivadia

@jeet-dekivadia jeet-dekivadia commented May 31, 2026

Copy link
Copy Markdown

Fixes #25.

Shared_writer_fd.close acquired the writer mutex but released it only when Unix.close raised. On the usual successful path, later writers waited forever before they could observe Closed.

Wrap the best-effort close in Fun.protect so the mutex is always released. The regression closes a shared writer and then checks that a later write raises Closed instead of blocking.

Validation:

  • git diff --check
  • private Linux runner: installed OCaml 5.2.1 and ppx_jane, then ran dune build @all -j2; compilation reaches the repository sources but the public toolchain cannot compile existing Domain.Safe and Sys.Safe uses and existing warning-as-error findings, so dune runtest is not reachable outside the Jane Street build environment

Signed-off-by: Jeet Dekivadia <jeet.university@gmail.com>
@github-iron github-iron added the forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system. label Jun 1, 2026
@jeet-dekivadia

Copy link
Copy Markdown
Author

Small follow-up here: this is scoped to the close path after the writer mutex is acquired. On the successful Unix.close path the mutex was not released, so later writers could block before observing Closed.

The patch wraps the close attempt in Fun.protect, preserving the existing best-effort close behavior while making unlock unconditional. The regression checks that a later write raises Closed instead of hanging. DCO is green.

@jeet-dekivadia

Copy link
Copy Markdown
Author

I rechecked this today while it is waiting for review. The ownership point is the writer mutex after Shared_writer.close has acquired it: successful Unix.close used to leave the mutex held, so later writes could wait instead of observing Closed.

The patch only makes unlock unconditional around the existing close attempt; it does not change the closed-state transition or the best-effort close behavior. DCO is still green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shared_writer_fd.close leaves its mutex locked after a successful close

2 participants