Skip to content

Database executor pool config is dead: ThreadPoolExecutor(1, 8, unbounded queue) can never exceed 1 thread #64

Description

@DiamondDagger590

Context

From the July 2026 McRPG quest-system audit (verified: CONFIRMED, severity medium — but load-bearing). Every DB operation in the McRPG quest vertical (player loads, chain saves, board rotation persistence, quest-tree saves, GUI history loads) serializes through a single thread, and at least one downstream component's correctness silently depends on that accident.

Problem

Database creates ThreadPoolExecutor(1, 8, 30s, new LinkedBlockingDeque<>()) (src/main/java/com/diamonddagger590/mccore/database/Database.java:43-44). With an unbounded work queue, a ThreadPoolExecutor never creates threads beyond corePoolSizemax=8 is unreachable dead configuration.

Two faces:

  • Performance: a slow batch (e.g. board-rotation persistence of hundreds of offerings) queues every player load behind it — join stalls during login waves.
  • Hidden correctness crutch: McRPG's ChainPersistenceService claims per-player write serialization but its future-chaining doesn't actually order execution — ordering holds only because this pool is single-threaded. Naively "fixing" this pool would introduce data races downstream.

Implementation plan

  1. Decide the contract explicitly and document it on getDatabaseExecutorService():
    • Option A (recommended for SQLite, the only shipped driver): make it an honest Executors.newSingleThreadExecutor() with Javadoc stating FIFO single-threaded execution is the contract downstream code may rely on.
    • Option B: real parallelism (bounded queue / fixed pool) — only after the McRPG chain-persistence ordering fix lands (companion McRPG ticket), and with Javadoc stating that submission order is NOT execution order.
  2. Either way, name threads (McCore-Database-Thread-%d) for diagnosability, and add an UncaughtExceptionHandler/wrapper so RuntimeExceptions thrown by submitted tasks are logged instead of vanishing into discarded Futures (the audit found multiple bare executor.submit() sites downstream whose failures disappear).
  3. Update CLAUDE.md (Database section) with the decided contract.

Acceptance criteria

  • The executor's concurrency contract is explicit in Javadoc and true in code (no dead config).
  • Uncaught exceptions in submitted tasks reach the plugin logger.
  • Unit test: tasks submitted from multiple threads execute under the documented ordering contract (use ManagedExecutorExtension).
  • ./gradlew test passes with zero failures.

Dependencies

If Option B is chosen: blocked by the McRPG ticket "Make ChainPersistenceService per-player write ordering real" (referenced from the McRPG tracking issue).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions