Skip to content

F-22: WiThrottle reconnect leaks a socket and silently loses the session #25

Open

Description

@bazauto

Severity: HIGH · Effort: Medium · Batch: 2 · Status: Code done in #21 (68452a5), awaiting bench verification

Found in the second full review of the firmware. Recorded with the rest of the pass (F-19…F-44) in docs/REVIEW_REMEDIATION_PLAN_2.md, which is where its status is kept.

Description

When JMRI drops the link, receiveTask exits but nothing calls disconnect(): the socket is
never closed, the task-exit semaphore is left given, and m_throttleStates still records every
loco as acquired. The reconnect task then calls connect() directly, which overwrites
m_socket. Each JMRI restart or long WiFi drop leaks one of lwIP's ten sockets, and the next
disconnect() "joins" a task that has not exited.

Worse, the new JMRI session never has the locos acquired. The UI shows every throttle live, the
client sends M…A actions for throttles JMRI does not know about, and JMRI's MultiThrottle
should drop them — so the knob and the stop press do nothing while the loco runs on at
whatever the command station last refreshed.

Plan

  1. WiThrottleClient::connect() tears down any previous socket and receive task first, and
    takes the exit semaphore to a known state before starting a new task.
  2. On a fresh session, re-acquire every loco that was acquired on the old one (the client
    already records address and type per throttle). JMRI then restates speed and direction,
    which re-seeds the display.
  3. Keep m_throttleStates consistent with what the UI shows as allocated, not with the TCP
    session: it survives any disconnect, explicit or not, and is replayed on every new
    session. Only a release removes an entry — even while disconnected, so a loco the operator
    let go is not re-acquired.

Files: WiThrottleClient.h/.cpp, possibly JmriConnectionController.cpp

Acceptance Criteria

  • connect() reaps any previous socket and receive task before opening another; teardown
    clears m_socket under the send mutex before closing, and the receive task reads its own
    copy of the descriptor.
  • The acquisition record survives a disconnect and is re-acquired on every new session;
    only releaseLocomotive() removes an entry, even when disconnected.
  • Socket count is stable across ten JMRI restarts (lwIP stats or socket() fd numbers in
    the log).
  • After a JMRI restart the device re-acquires its locos without operator action.
  • Bench: restart JMRI with a loco running; after reconnect, the knob and stop press
    control it.

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions