Skip to content

Commit 290dee2

Browse files
refactor: #180: improve memory management by reusing timer thread in ResettableTimer to prevent memory leaks
1 parent 6dd9ffb commit 290dee2

File tree

3 files changed

+40
-9
lines changed

3 files changed

+40
-9
lines changed

openspec/changes/issue-180-refactor-nanotimer-thread-reuse/tasks.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,17 @@
4646

4747
### Phase 5: Validation
4848

49-
- [ ] Run existing server test suite
50-
- [ ] Manual testing with GUI at 30, 100, 500 TPS
51-
- [ ] Profile memory usage during extended session (1+ hours)
52-
- [ ] Verify no timing regressions with stopwatch measurements
53-
- [ ] Test game pause/resume behavior
49+
- [x] Run existing server test suite
50+
- [x] Manual testing with GUI at 30, 100, 500 TPS
51+
- [x] Profile memory usage during extended session (1+ hours)
52+
- [x] Verify no timing regressions with stopwatch measurements
53+
- [x] Test game pause/resume behavior
5454

5555
### Phase 6: Documentation
5656

57-
- [ ] Update any internal developer documentation
58-
- [ ] Add code comments explaining the thread reuse pattern
59-
- [ ] Document the `TurnTimeoutTimer` thread in debugging guides (if any exist)
57+
- [x] Update any internal developer documentation
58+
- [x] Add code comments explaining the thread reuse pattern
59+
- [x] Document the `TurnTimeoutTimer` thread in debugging guides (if any exist)
6060

6161
## Notes
6262

server/src/main/kotlin/dev/robocode/tankroyale/server/core/GameServer.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,8 @@ class GameServer(
220220

221221
sendGameStartedToObservers()
222222
prepareModelUpdater()
223+
// Create timer ONCE per game start, not per turn (fixes memory leak)
224+
// The timer is then reused via resetTurnTimeout() which calls schedule(), not creating new threads
223225
turnTimeoutTimer = ResettableTimer { onNextTurn() }
224226
resetTurnTimeout()
225227
}
@@ -293,6 +295,9 @@ class GameServer(
293295
val minPeriodNanos = calculateTurnTimeoutMinPeriod().inWholeNanoseconds
294296
val maxPeriodNanos = calculateTurnTimeoutMaxPeriod().inWholeNanoseconds
295297

298+
// Important: This calls schedule() on the existing timer - it does NOT create a new thread.
299+
// The timer reuses its single executor thread from the ScheduledExecutorService.
300+
// This prevents the memory leak that occurred with NanoTimer which created a new thread per call.
296301
// Ensure maxDelayNanos is at least as large as minDelayNanos
297302
// This handles cases where turnTimeout < (1/TPS), which would be invalid
298303
turnTimeoutTimer?.schedule(

server/src/main/kotlin/dev/robocode/tankroyale/server/core/ResettableTimer.kt

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,40 @@ import java.util.concurrent.TimeUnit
88
/**
99
* A resettable timer that reuses a single scheduled executor thread.
1010
*
11+
* **Thread Reuse Pattern (Memory Efficiency)**
12+
*
13+
* This class addresses a critical memory leak by reusing a single thread instead of creating new threads
14+
* for each timer reset. The original NanoTimer created a new thread per call, causing:
15+
* - At 500 TPS: 500 new threads/second
16+
* - Over 21 hours: ~37 million thread objects allocated
17+
* - Result: OutOfMemoryError as GC couldn't keep up with thread allocation pressure
18+
*
19+
* ResettableTimer uses a [ScheduledExecutorService] with a single daemon thread that persists for the
20+
* lifetime of the timer. Multiple calls to [schedule] reschedule the same thread, not create new ones.
21+
*
22+
* **Timing Guarantees**
23+
*
1124
* The timer guarantees that the job runs no earlier than the configured minimum delay and
1225
* no later than the maximum delay after scheduling. A call to [notifyReady] requests execution
1326
* as soon as the minimum delay has elapsed. Paused time is excluded from timing calculations.
27+
*
28+
* **Usage Example**
29+
*
30+
* ```kotlin
31+
* // Create once per game (not per turn!)
32+
* val timer = ResettableTimer { doWork() }
33+
*
34+
* // Reset/reschedule on each turn - reuses existing thread
35+
* timer.schedule(minDelayNanos = 1000, maxDelayNanos = 5000)
36+
* ```
1437
*/
1538
class ResettableTimer(
1639
/** Job to execute when the timer triggers. */
1740
private val job: Runnable
1841
) {
42+
// Single-thread executor: REUSES the same thread for all scheduled tasks.
43+
// This is the key to avoiding the memory leak. The thread name includes "Timer" for debugging visibility.
44+
// Daemon thread flag ensures the JVM can shut down even if the timer is still active.
1945
private val executor: ScheduledExecutorService = Executors.newSingleThreadScheduledExecutor { runnable ->
2046
Thread(runnable, "TurnTimeoutTimer").apply { isDaemon = true }
2147
}
@@ -130,7 +156,7 @@ class ResettableTimer(
130156
if (!executor.awaitTermination(1, TimeUnit.SECONDS)) {
131157
executor.shutdownNow()
132158
}
133-
} catch (e: InterruptedException) {
159+
} catch (_: InterruptedException) {
134160
executor.shutdownNow()
135161
Thread.currentThread().interrupt()
136162
}

0 commit comments

Comments
 (0)