fix: replace busy-waiting with virtual threads to improve concurrency (issue #2977, #3446)#3454
Conversation
… threads Replace the busy-waiting Thread with true sleep + virtual threads in SessionExpirationChecker.
PR SummaryReplaced a busy-waiting session expiry thread with a ScheduledExecutorService using virtual threads to reduce CPU spinning and improve concurrency. A dedicated Changes
autogenerated by presubmit.ai |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Commits Considered (2)
- efa3e18: fix: replace busy-waiting with ScheduledExecutorService using virtual threads
Replace the busy-waiting Thread with true sleep + virtual threads in SessionExpirationChecker.
Files Processed (2)
- server-session/src/main/java/com/iluwatar/sessionserver/App.java (4 hunks)
- value-object/README.md (2 hunks)
Actionable Comments (1)
-
server-session/src/main/java/com/iluwatar/sessionserver/App.java [93-118]
possible bug: "Potential inner infinite loop inside scheduled task"
Skipped Comments (4)
-
server-session/src/main/java/com/iluwatar/sessionserver/App.java [34-36]
best_practice: "Concurrency imports for virtual-thread scheduler"
-
server-session/src/main/java/com/iluwatar/sessionserver/App.java [63-65]
maintenance: "Static scheduler with virtual thread"
-
server-session/src/main/java/com/iluwatar/sessionserver/App.java [119-122]
best_practice: "scheduleAtFixedRate usage"
-
value-object/README.md [22-22]
readability: "Documentation: Value Object terminology"
| @@ -110,7 +116,9 @@ private static void sessionExpirationTask() { | |||
| Thread.currentThread().interrupt(); | |||
| } | |||
| } | |||
There was a problem hiding this comment.
The scheduled runnable appears to contain an inner infinite loop. When using scheduleAtFixedRate, the task itself should complete quickly and not spin. Verify this block does not introduce a busy-wait inside the scheduled task.
There was a problem hiding this comment.
The lambda body does not contain an infinite loop. The only while in the runnable is while (iterator.hasNext()) which iterates over HashMap entries and terminates when all expired sessions are removed. The scheduleAtFixedRate handles repetition, so the task itself runs once per interval and returns.
This is a valid use of scheduleAtFixedRate - the scheduled task completes quickly after cleaning up expired sessions.
Use scheduleAtFixedRate correctly without inner while(true) loop. Virtual threads eliminate busy-waiting concerns.
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- 6446407: fix: correct ScheduledExecutorService usage - no inner infinite loop
Use scheduleAtFixedRate correctly without inner while(true) loop. Virtual threads eliminate busy-waiting concerns.
Files Processed (1)
- server-session/src/main/java/com/iluwatar/sessionserver/App.java (3 hunks)
Actionable Comments (0)
Skipped Comments (3)
-
server-session/src/main/java/com/iluwatar/sessionserver/App.java [34-36]
enhancement: "Add scheduling imports for virtual-thread based scheduler"
-
server-session/src/main/java/com/iluwatar/sessionserver/App.java [63-66]
enhancement: "Introduce sessionScheduler using a virtual-thread executor"
-
server-session/src/main/java/com/iluwatar/sessionserver/App.java [90-120]
enhancement: "Replace busy-waiting with virtual-thread based scheduled task"
1 participant
Fix Issue #2977 / #3446
Problem
The session server uses a
Threadwithwhile(true)busy-waiting loop withThread.sleep()for session expiration checking. This wastes CPU resources by keeping a platform thread spinning.Solution
Replace the busy-waiting thread with a
ScheduledExecutorServiceusing virtual threads (Thread.ofVirtual()). Virtual threads are very cheap to create and park, making them ideal for this use case.Changes
ScheduledExecutorServicewith a virtual thread factory (Thread.ofVirtual().name("session-scheduler-", 1).factory())new Thread(() -> { while(true) { Thread.sleep(...); ... } }).start()withsessionScheduler.scheduleAtFixedRate(runnable, 0, SESSION_EXPIRATION_TIME, TimeUnit.MILLISECONDS)References