Skip to content

fix(server): make RustInterface.serverStarted volatile - #341

Closed
0xbrayo wants to merge 1 commit into
ActivityWatch:masterfrom
0xbrayo:fix/server-started-volatile
Closed

0xbrayo wants to merge 1 commit into
ActivityWatch:masterfrom
0xbrayo:fix/server-started-volatile

Conversation

@0xbrayo

@0xbrayo 0xbrayo commented Oct 8, 2026

Copy link
Copy Markdown
Member

Part of #333.

RustInterface.serverStarted is shared across threads:

  • the IO coroutine that starts the server sets it to true,
  • a main-thread Handler sets it to false when the server exits,
  • BackgroundService's sanitized-hostname-rewrite thread polls it in a while (RustInterface.serverStarted) Thread.sleep(...) loop.

Without @Volatile, the Java memory model doesn't guarantee that the polling thread ever sees the main thread's write, so the queued hostname rewrite could wait forever.

Change

  • Mark the field @Volatile.

Testing

  • ./gradlew :mobile:testStandardDebugUnitTest passes.

@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

Adds @Volatile to RustInterface.serverStarted so the hostname-rewrite thread can see when the server exits.

  • The new comment explains why the field needs cross-thread visibility.
  • No actionable issues found.

Reviews (1) · Last reviewed commit: "fix(server): make RustInterface.serverSt..." · Reviewed by Greptile

The flag is written on the main thread when the server exits and polled
in a loop by the hostname-rewrite thread, which without @volatile is not
guaranteed to ever see the change.
@0xbrayo
0xbrayo force-pushed the fix/server-started-volatile branch from f77c6b5 to 9e5d9e3 Compare October 8, 2026 19:37
@ErikBjare

Copy link
Copy Markdown
Member

Thanks @0xbrayo. Master already has this: #324 made serverStarted @Volatile (with the startServerTask() check-and-set under a lock), and its comment calls out the hostname-rewrite reader in BackgroundService. Rebasing this branch leaves an empty diff, so I'm closing it as superseded by #324.

@ErikBjare ErikBjare closed this Oct 11, 2026
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