Skip to content

chore(ligretto): connection-lifecycle state hygiene (user records, dead removeUser API, socket retention in timers) #656

Description

@Themezv

Context

Three related memory/state-hygiene findings from the PR #648 code review, all confirmed by verification but low-severity (in-memory single process, restarted on deploys). Grouped here as one cleanup task.

1. storage.users grows without bound

UserService.disconnectionHandler keeps every user record forever (deliberately, so a reconnect can restore identity), but nothing ever reaps them:

  • users who connect and browse the lobby without joining a room are never deleted;
  • users who explicitly leave a room have currentGameId cleared by leaveFromRoomHandler before the only sweep (GameRepository.deleteIfAllParticipantsOffline, which requires currentGameId === deletedGameId) could ever match them;
  • endGame / leaveGame → removeGame paths do not touch users at all.

Growth is proportional to lifetime unique visitors (entries are tiny). Fix ideas: an idle-user sweep (no sockets + no currentGameId), or delete the record at the points where the last association ends.

2. Dead removeUser API

UserService.removeUser and UserRepository.removeUser have zero callers (production and tests) since the lifecycle rewrite, while deleteIfAllParticipantsOffline deletes user records by hand with a raw delete storage.users[userId]. Either wire the sweep from item 1 through removeUser, or delete the API — the current state suggests eviction is handled somewhere when it is not.

3. Dead sockets retained by lifecycle timers

games-controller.lifecycleEvents(socket) closures are captured by the grace (5s), host-handover (30s) and room-deletion (120s) timers in GameConnectionService. A disconnected Socket keeps its handshake (headers/auth), data, client and adapter references after close, so each armed timer pins one dead socket for up to ~2 minutes (bounded: re-arming overwrites, so it is O(games), not O(sockets); verification rated the magnitude negligible for this app).

Cleaner shape: inject the Socket.IO server/namespace (via IOC, not the module-level io export — importing socket-io-server starts the HTTP server and would break unit tests) and broadcast lifecycle events through io.to(room) / io.to(SOCKET_ROOM_LOBBY). That removes the per-disconnect closure allocation and the odd pattern of broadcasting through a dead socket, and GameConnectionService timer callbacks stop needing an events parameter altogether.

Found during the PR #648 code review; none of these block the PR.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions