Skip to content

FcmPushClient.stop() closes the socket and awaits tasks on shutdown - #41

Open
drewbuerger wants to merge 1 commit into
sdb9696:mainfrom
drewbuerger:fix/stop-clean-shutdown
Open

drewbuerger wants to merge 1 commit into
sdb9696:mainfrom
drewbuerger:fix/stop-clean-shutdown

Conversation

@drewbuerger

Copy link
Copy Markdown

stop() cancels the listen/monitor tasks but never closes the socket or awaits
them, so a _listen() task parked on reader.readexactly() hangs on the SSL
read. Downstream consumers such as Home Assistant's Ring integration then log
Task could not be canceled and was still running after shutdown and block
~15s on every shutdown.

  • stop(): cancel the tasks, close the writer so the read unblocks, then await
    the tasks (bounded by STOP_TIMEOUT_SEC).
  • stop(): return cleanly when called before start() (stopping_lock is
    None) — previously raised TypeError on async with None.
  • _terminate(): close the transport so a sibling reader unblocks on
    connectivity loss (related to Internet connectivity dropout causing irrecoverable shutdown #33).

Adds tests/test_stop_shutdown.py using a loopback socket pair (no FCM servers
needed). The old stop() fails or hangs these.

stop() cancelled the listen/monitor tasks but never closed the socket or
awaited them, so a _listen() task parked on reader.readexactly() hangs on
the ssl read. Callers such as Home Assistant then log "Task could not be
canceled and was still running after shutdown" and block ~15s per stop.

- stop(): cancel tasks, close the writer so the read unblocks, then await
  the tasks (bounded by STOP_TIMEOUT_SEC)
- stop(): return cleanly when called before start() (stopping_lock is None),
  which previously raised TypeError on 'async with None'
- _terminate(): close the transport so a sibling reader unblocks (see sdb9696#33)

Adds tests/test_stop_shutdown.py using a loopback socket pair (no FCM
servers); the old stop() fails or hangs these.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant