Skip to content

Fix InputStreamMonitor.writeNext() wait() not in loop - #2882

Merged
vogella merged 1 commit into
eclipse-platform:masterfrom
robstryker:issue-2881
Sep 22, 2026
Merged

vogella merged 1 commit into
eclipse-platform:masterfrom
robstryker:issue-2881

Conversation

@robstryker

Copy link
Copy Markdown
Contributor

Summary

  • Change if (fQueue.isEmpty()) to while (fQueue.isEmpty() && !fClosed) around fLock.wait() in writeNext() to guard against spurious wakeups per JLS 17.2.1
  • Also checks !fClosed to avoid waiting on a closed stream

The existing if guard was added for bug 550834 but doesn't protect against spurious wakeups. Found via SpotBugs static analysis (WA_NOT_IN_LOOP).

Fixes #2881

Test plan

  • Existing InputStreamMonitorTests pass
  • Manual verification: launch a process, write to its stdin, verify output

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Test Results

    54 files  +  3      54 suites  +3   57m 7s ⏱️ + 2m 9s
 4 811 tests ±  0   4 789 ✅ +  1   22 💤 ±0  0 ❌  - 1 
12 324 runs  +261  12 171 ✅ +262  153 💤 ±0  0 ❌  - 1 

Results for commit 0b4f8ff. ± Comparison against base commit 6e4cd9b.

♻️ This comment has been updated with latest results.

@laeubi laeubi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@vogella
vogella force-pushed the issue-2881 branch 2 times, most recently from 016cc7d to cf249d3 Compare September 2, 2026 09:35
vogella
vogella previously requested changes Sep 3, 2026

@vogella vogella left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added !fClosed check is wrong and introduces a loop. After the console EOF action calls closeInputStream(), the monitor thread keeps running until the process exits, and with this change writeNext() no longer waits but returns immediately, so the while (fThread != null) loop in write() spins at full CPU.

Existing tests pass because none of them keep the thread alive after closeInputStream() but our production code does that.

Correct would be while (fQueue.isEmpty())

robstryker added a commit to redhat-developer/rsp-server that referenced this pull request Sep 21, 2026
Remove !fClosed from the wait loop condition. After closeInputStream()
sets fClosed, the condition caused writeNext() to return immediately,
making the writeLoop() spin at full CPU until close() sets fThread
to null.

See: eclipse-platform/eclipse.platform#2882 (review)
Fixes #764

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vogella

vogella commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Please squash both commits into one.

Use while loop instead of if to guard Object.wait() against spurious
wakeups per JLS 17.2.1.

Fixes eclipse-platform#2881
@robstryker

Copy link
Copy Markdown
Contributor Author

Changes were made and squashed

@vogella
vogella merged commit 40a1c57 into eclipse-platform:master Sep 22, 2026
18 checks passed
@vogella

vogella commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Thanks

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.

InputStreamMonitor.writeNext() wait() not in loop — vulnerable to spurious wakeups

3 participants