Report writes the host refuses by Project516 · Pull Request #19 · Project516/java-web

Project516 · 2026-11-02T16:53:27Z

java-web buffers a file's writes and writes them out on flush or close. When the host refused them (disk full, file too large, quota), the bytes were dropped and close(), FileDescriptor.sync() and FileChannel.force() still returned normally, so a program believed a truncated file was complete.

flush returns whether the host took the bytes, and the host backend keeps the errno of the last failed write per path.
close (FileDescriptor and channel closes), sync0 and force0 throw IOException with the errno's message, such as "File too large" or "No space left on device". The descriptor is still closed, as with close(2), and the Java FileDescriptor is invalidated before the close can throw. MappedByteBuffer.force() reports a failed write-back the same way; unmapping does not, as munmap does not. Implicit flushes (a stat or open of the same path) keep the bytes buffered and retry later instead of throwing.

HotSpot reports such an error from write(); java-web reports it at the next flush, sync, force or close, so code between the write and the close still runs. A program that wraps the write and the close in one try, as most do, sees the same exception.

A new test runs a program under ulimit -f 4 (with SIGXFSZ ignored, writes past 4 KB fail with EFBIG) and compares close, sync and force with HotSpot; it fails without this change.

Verified: the new test, pnpm typecheck.

…d force instead of losing it.

coderabbitai · 2026-11-02T16:53:40Z

Important

Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: cb6b2527-b6f3-414b-b67c-7ff54d2991d9

Recent review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 91923187-27f6-4d86-b5e7-5ec892a1e4aa

Commits

Reviewing files that changed from the base of the PR and between dc3f078 and f86e7cc.

Files selected for processing: 6

src/fs.ts
src/natives/io.ts
src/natives/nio.ts
src/node.ts
test/full/Full.java
test/programs.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Recent review details

Context from checks skipped due to timeout: 5

GitHub Check: jtreg / run (4)
GitHub Check: jtreg / run (3)
GitHub Check: jtreg / run (1)
GitHub Check: jtreg / run (2)
GitHub Check: CodeRabbit

Additional context used

Learnings: 1

Common learnings

Learnt from: CR
Repo: Project516/java-web PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-11-01T20:52:55.551Z
Learning: Source excerpt:
# java-web

## Direction

- Prove it with tests: programs whose output must match HotSpot exactly, and
  OpenJDK's own regression tests where they can run.

Learnt from: CR
Repo: Project516/java-web PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-11-01T20:52:55.551Z
Learning: Source excerpt:
# java-web

## Direction

- Compatibility first. Run JDK classes unmodified and implement what HotSpot
  does for them (natives, VM upcalls, init order). Do not patch or replace JDK
  classes. When behavior differs from HotSpot, HotSpot is right.

ast-grep (0.50.0)

src/node.ts

[warning] 74-74: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(this.host(p), data)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

GitHub Check: CodeQL

test/programs.test.ts

[warning] 85-85: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.

Summary

Summary by CodeRabbit

Bug Fixes
- File write failures are now reported when buffered data is flushed, including when closing a file or syncing it through file descriptors and channels. This helps prevent unsuccessful writes from appearing to complete normally.
- Error reports now include the filesystem’s available error information, with a general I/O error used when a specific cause cannot be provided.

The filesystem backend now exposes the error code from a failed write. Flush operations report write failures, and close, sync, and force paths surface them as I/O errors. A Linux-only test compares these cases with HotSpot.

Changes: Write Failure Propagation

Layer / File(s)	Summary
Backend write error reporting `src/fs.ts`, `src/node.ts`	`Backend` and `VFS` expose the backend’s write error code. `NodeBackend` records the error code when a write fails.
Flush error handling `src/natives/io.ts`, `src/natives/nio.ts`	`flush` returns whether buffered writes succeed. `closeFd`, `FileDescriptor.sync0`, and `force0` handle flush failures by throwing I/O errors.
Write failure parity test `test/full/Full.java`, `test/programs.test.ts`	The Java test exercises writes followed by close, sync, or force. The Linux-only test compares output and exit status with HotSpot under a 4 KB file-size limit.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

Project516/java-web#8: Adds the Backend.chmod contract and NodeBackend.chmod implementation, which PR #19 uses in its flush path.
Project516/java-web#75: Changes closeFd for socket descriptors and retains the file-descriptor flush path for non-socket descriptors.

Suggested reviewers: project516-review-bot

Merge Risk: Moderate · previous review f86e7

The change makes failed host writes surface as IOException in most close, sync and force paths. Mapped-buffer force can still report success after a refused write, and a descriptor whose close failed can later report a successful sync. Both leave silent data-loss paths in the feature this PR adds, so they should be fixed before merge.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title identifies the main change: reporting writes that the host refuses. Although the wording is slightly awkward, it is specific and related to the changeset.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description explains the change in detail. It says java-web buffers file writes and drops them on failure, and that close, FileDescriptor.sync and FileChannel.force returned normally. It also cove…

Full details: Description check

Explanation

The description explains the change in detail. It says java-web buffers file writes and drops them on failure, and that close, FileDescriptor.sync and FileChannel.force returned normally. It also covers the fix. flush returns whether the host accepted the bytes, the host backend keeps the errno of the last failed write per path, and close, sync0 and force0 throw IOException with the errno message. It notes the behavior differences from HotSpot and the new ulimit-based test. This matches the changeset in src/fs.ts, src/natives/io.ts, src/natives/nio.ts, src/node.ts and the new test files.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 6 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

Commit to this branch
Create a new PR

Simplify code

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

_{A rabbit taps the buffer twice,
Then checks the write and names the price.
If bytes cannot make it through,
An I/O error comes into view.
Close, sync, and force all tell,
While test runs match HotSpot well.}

project516-review-bot

The PR propagates host write errors (ENOSPC, EFBIG, EDQUOT, etc.) through flush, sync, force, and close by making flush return a boolean, adding flushOrThrow that throws IOException with the errno message, and wiring it into FileDescriptor.sync0, FileDescriptor.close0, and FileChannel.force0. A new test runs under ulimit -f 4 and matches HotSpot output.

_{review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict approve}

coderabbitai

Fix comments: Fix CodeRabbit comments on this PR

…uffer.force, and invalidate a descriptor before a close that can throw.

project516-review-bot

The PR adds proper error propagation for failed writes (disk full, file too large, quota). Buffered writes now report errors at explicit flush/sync/force/close via IOException with the correct errno message, while implicit flushes (stat, open, rename) keep bytes buffered and retry later. The Backend interface and NodeBackend now track write errors per path using a Map, clearing on successful write. A new test verifies behavior matches HotSpot under ulimit -f. Earlier review concerns about cross-file error contamination have been addressed: NodeBackend.write clears the error on success (line 75), and the Backend.writeError method takes a path parameter for per-file tracking.

Not approved: 3 earlier points are still open in the threads.

_{review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict comment}

project516-review-bot

All points from the last review are settled in the threads.

_{review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict approve}

Report a write the host refuses as an IOException from close, sync an…

f86e7cc

…d force instead of losing it.

github-advanced-security Bot found potential problems Nov 2, 2026

View reviewed changes

Comment thread test/programs.test.ts Dismissed

project516-review-bot Bot approved these changes Nov 2, 2026

View reviewed changes

Comment thread src/node.ts Resolved

Comment thread src/fs.ts Resolved

Comment thread src/fs.ts Resolved

coderabbitai Bot requested changes Nov 2, 2026

View reviewed changes

Comment thread src/natives/io.ts Resolved

Comment thread src/natives/io.ts Resolved

Keep write errors per path, report failed write-back from MappedByteB…

a894b4a

…uffer.force, and invalidate a descriptor before a close that can throw.

project516-review-bot Bot reviewed Nov 2, 2026

View reviewed changes

project516-review-bot Bot approved these changes Nov 2, 2026

View reviewed changes

Project516 merged commit 083ffa2 into master Nov 2, 2026
12 checks passed

Project516 deleted the p516/flush-errors branch November 2, 2026 17:43

coderabbitai Bot mentioned this pull request Nov 4, 2026

Report the host's reason when a new file cannot be created #35

Merged

Navigation Menu

Repository navigation

Report writes the host refuses - #19

Report writes the host refuses#19
Project516 merged 2 commits into
masterfrom
p516/flush-errors

Project516 commented Nov 2, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Nov 2, 2026 •

edited

Loading

Summary by CodeRabbit

Uh oh!

Uh oh!

project516-review-bot Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

project516-review-bot Bot left a comment

Uh oh!

project516-review-bot Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Project516 commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

Uh oh!

project516-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

project516-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

project516-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Project516 commented Nov 2, 2026 •

edited

Loading

coderabbitai Bot commented Nov 2, 2026 •

edited

Loading

coderabbitai Bot left a comment •

edited

Loading