fix(mp_logger): deterministic logger socket drain on shutdown (Phase-3 R5)

[vringar]

What

Replaces the time.sleep(3) wait-and-hope in MPLogger._start_listener with a deterministic close-and-await-drain of the logging ServerSocket, so no log records are lost on shutdown and there is no fixed-duration stall.

This is Phase-3 R5 of the socket-reliability thread (the timeout audit's one clean, design-fork-free REPLACE item). It is self-contained and independent of the bidi extension back-channel.

The problem

On shutdown, MPLogger closed the listening socket and then slept 3 seconds, hoping any in-flight client messages had landed in socket.queue before draining it:

socket.close()
time.sleep(3)  # TODO: the socket needs a better way of closing
while not socket.queue.empty():
    ...

socket.close() only closes the listening socket; the per-connection handler threads stay blocked in recv() on their own client sockets. Too short ⇒ tail log records dropped; too long ⇒ shutdown stalls.

The drain handshake

Added ServerSocket.shutdown(timeout=None) to socket_interface.py, which makes the drain deterministic:

1. Set a _shutting_down flag and make a throwaway self-connection to wake the accept() loop (closing the listening socket alone does not reliably unblock a thread parked in accept()), then join the accept thread. No new connections are accepted past this point.
2. Close the listening socket.
3. shutdown(SHUT_RDWR) every live client socket, forcing each handler thread blocked in recv() to return and exit.
4. join all handler threads.

ServerSocket now tracks its accept thread, its per-connection handler threads, and the live client sockets to make this possible. Each handler puts every fully-received message onto self.queue before exiting, so once shutdown() returns the queue holds every such message — the caller drains it with no risk of losing a tail record and no fixed wait.

MPLogger._start_listener now calls socket.shutdown() then drains; the sleep(3) and the now-unused time import are gone.

Tests

• New test/test_socket_interface.py:
  • test_shutdown_drains_all_sent_records — 500 messages sent then client closed; all 500 present after shutdown().
  • test_shutdown_with_open_client_connection — client left open; shutdown() still drains and all threads exit (this caught a real hang where the accept thread did not wake on a bare close()).
  • test_shutdown_no_clients — drain is safe when nothing connected.
• Existing test/test_mp_logger.py (4 tests) still pass; shutdown completes promptly.

pytest test/test_socket_interface.py test/test_mp_logger.py → 7 passed. pre-commit (isort/black/mypy) green.

Browser-dependent tests are env-blocked locally (FF150 vs 152) and left to CI.

Scope

Does not touch R1–R4 from the timeout audit (those are needs-human / depend on the bidi back-channel).

[codecov]

Codecov Report

❌ Patch coverage is 76.92308% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.33%. Comparing base (315e667) to head (5f8d145).

Files with missing lines	Patch %	Lines
openwpm/socket_interface.py	76.47%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1199      +/-   ##
==========================================
+ Coverage   62.14%   62.33%   +0.18%     
==========================================
  Files          40       40              
  Lines        3918     3961      +43     
==========================================
+ Hits         2435     2469      +34     
- Misses       1483     1492       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.