`ExecProcess.wait()`/`wait_output()` leak non-daemon I/O threads when `wait_change` raises (e.g. exec timeout)

[skourta]

I first noticed the issue in the OpenSerach k8s charm where the charm just hangs indefinitely on a hook.

Disclaimer: I generated the issue description with AI

Summary

When an ExecProcess's underlying change wait fails — most commonly when an exec
timeout elapses, but also on any error from wait_change() — ExecProcess._wait()
propagates the exception before it joins its I/O pump threads and shuts down the
exec websockets. Because those pump threads are created with the default
daemon=False, they remain blocked in websocket.recv() on websockets that are
never closed.

This has two consequences:

1. Interpreter shutdown hangs. threading._shutdown() blocks forever joining the
   still-running non-daemon threads (observed as a hang in lock.acquire() at process
   exit).
2. Noisy unraisable tracebacks. If/when the threads eventually unblock (peer closes
   the connection), they raise
   websocket._exceptions.WebSocketConnectionClosedException: socket is already closed.
   to threading.excepthook, printing full tracebacks from
   shutil.copyfileobj → ops.pebble._WebsocketReader.read → ws.recv().

Root cause

In ExecProcess._wait() (ops/pebble.py ~L1784):

def _wait(self) -> int:
    self._waited = True
    timeout = self._timeout
    if timeout is not None:
        timeout += 1
    change = self._client.wait_change(self._change_id, timeout=timeout)   # L1790  <-- can raise

    # --- cleanup below only runs if wait_change() returned normally ---
    if self._cancel_stdin is not None:        # L1793
        self._cancel_stdin()
    for thread in self._threads:              # L1797
        thread.join()
    if self._cancel_reader is not None:       # L1802
        os.close(self._cancel_reader)
    self._control_ws.shutdown()               # L1806
    self._stdio_ws.shutdown()
    if self._stderr_ws is not None:
        self._stderr_ws.shutdown()
    ...

self._client.wait_change(...) at L1790 raises on the client-side websocket timeout
(timeout + 1) when the server doesn't complete the change in time. The exception
propagates straight out of _wait(), so the cleanup at L1793–L1809 (cancel stdin,
join threads, shutdown websockets) is skipped.

The leaked threads are:

• the shutil.copyfileobj pumps started in wait_output() (L1845–L1850), and
• the forwarder threads started in Client.exec()
  (_reader_to_websocket / _websocket_to_writer, ~L3172–L3197).

They are started via _start_thread() (L412–L416):

def _start_thread(target, *args, **kwargs):
    thread = threading.Thread(target=target, args=args, kwargs=kwargs)  # daemon=False
    thread.start()
    return thread

Both wait() (L1769) and wait_output() (L1819) are affected, since both delegate to
_wait().

Reproduction

container.exec(["sleep", "60"], timeout=5, combine_stderr=True, encoding="utf-8").wait_output()

The exec server-side timeout / client-side wait_change timeout fires, wait_output()
raises, and the stdout copyfileobj thread is left blocked on self._stdio_ws. The
process then either hangs at interpreter exit (threads still blocked) or prints, when
they later unblock:

Exception in thread Thread-14 (copyfileobj):
Traceback (most recent call last):
...
File ".../ops/pebble.py", line 1982, in read
    chunk = self.ws.recv()
File ".../websocket/_core.py", line 395, in recv
...
File ".../websocket/_socket.py", line 95, in recv
    raise WebSocketConnectionClosedException("socket is already closed.")
websocket._exceptions.WebSocketConnectionClosedException: socket is already closed.

Impact

• Any consumer that runs exec(..., timeout=...) and handles the resulting exception
  can hang at process exit, or emit alarming-looking tracebacks from background threads
  that are actually benign teardown noise.
• This is particularly painful in Juju charm hooks: a single timed-out exec can wedge
  the hook process (Juju then has to kill it) or flood juju debug-log with thread
  tracebacks.

Suggested fix

Guarantee the teardown runs whether or not wait_change() succeeds — i.e. move the
cancel-stdin / join-threads / shutdown-websockets steps into a finally block in
_wait():

def _wait(self) -> int:
    self._waited = True
    timeout = self._timeout
    if timeout is not None:
        timeout += 1
    try:
        change = self._client.wait_change(self._change_id, timeout=timeout)
    finally:
        if self._cancel_stdin is not None:
            self._cancel_stdin()
        for thread in self._threads:
            thread.join()
        if self._cancel_reader is not None:
            os.close(self._cancel_reader)
        self._control_ws.shutdown()
        self._stdio_ws.shutdown()
        if self._stderr_ws is not None:
            self._stderr_ws.shutdown()
    ...

Note: thread.join() in the cleanup path is itself only safe to call after
the websockets are shut down (otherwise the join can block on a still-blocked
recv()); the team may want to shut the websockets down first, then join, in the
failure path. A simpler/more robust belt-and-braces option is to also create the I/O
pump threads as daemon=True so a missed teardown can never hang interpreter
shutdown.

Environment

• ops 3.7.1 (also present in 3.7.0; the relevant code is unchanged across recent 3.x)
• websocket-client 1.9.0
• Python 3.12 / 3.13

[tonyandrewmeyer]

I tried to reproduce this against the current head of main, with Pebble v1.31.0, since the description is mostly model generated and I wanted to verify it against the actual behaviour.

The reproduction in the description doesn't trigger the problem. With exec(['sleep', '60'], timeout=5) and a healthy Pebble, the server kills the command at the timeout and marks the change ready (with an error), so wait_change() returns normally, the teardown in _wait() runs, and you get a ChangeError and no leaked threads. That's the same with and without combine_stderr.

The leak itself is real, but it requires wait_change() to raise, which means the change has to still be running when the client gives up at timeout+1 (or the request has to fail in some other way, like a connection error). The simplest way I found to get there is a command that leaves an orphan holding the exec's stdout pipe open. Pebble kills the command at the timeout, but the change can't complete until the pipe closes, so the client raises ops.pebble.TimeoutError and the copyfileobj threads are left blocked in ws.recv(). That seems consistent with the OpenSearch case, where hooks will be exec'ing things that start or restart daemons.

With the script below, wait_output() raises after 3 seconds, both pump threads are still alive and non daemon, and the interpreter then hangs at shutdown until the orphan goes away (a timeout 30 wrapper has to SIGKILL the process). Reproduced on Python 3.10, 3.11, 3.12, 3.13, and 3.14.

repro.py (run a pebble daemon, then python repro.py $PEBBLE/.pebble.socket grandchild)

"""Reproduction attempt for canonical/operator#2556."""

import sys
import threading
import time

from ops import pebble

sock = sys.argv[1]
variant = sys.argv[2]

COMMANDS = {
    # The repro from the issue: pebble kills this at the timeout.
    'plain': ['sleep', '60'],
    # A child escapes the process group and keeps the stdout pipe open,
    # so the change can't complete even after pebble kills the main process.
    'grandchild': ['/bin/sh', '-c', 'setsid sleep 60 & sleep 60'],
}

client = pebble.Client(socket_path=sock)
combine = len(sys.argv) > 3 and sys.argv[3] == 'combine'
start = time.time()
try:
    process = client.exec(COMMANDS[variant], timeout=2, combine_stderr=combine)
    out, err = process.wait_output()
    print(f'NO EXCEPTION after {time.time() - start:.1f}s: out={out!r} err={err!r}')
except Exception as e:
    print(
        f'exception after {time.time() - start:.1f}s: '
        f'{type(e).__module__}.{type(e).__name__}: {e}'
    )

time.sleep(0.5)  # give threads a moment to finish naturally
extra = [t for t in threading.enumerate() if t is not threading.main_thread()]
print(f'non-main threads alive: {len(extra)}')
for t in extra:
    print(f'  {t.name} daemon={t.daemon} alive={t.is_alive()}')
print('main thread done, interpreter exiting now', flush=True)

One note on the suggested fix: moving the teardown into a finally block isn't enough, and would make this case worse. websocket-client's shutdown() only closes the file descriptor, and closing a socket in one thread doesn't wake another thread that's blocked in recv() on it, so the thread.join() calls would hang the hook immediately rather than at exit. Unblocking the threads needs an OS level shutdown(SHUT_RDWR) on the socket first, and the reader threads then need to treat the lost connection as EOF rather than crashing with the traceback quoted above. I'm working on a PR.

[skourta]

Thank you @tonyandrewmeyer I didn't trust the generated fix so I left a disclaimer.
I have added a workaround for this on our charm currently while waiting for a fix on pebble.
Thank you for being fast to debug and confirm.

[tonyandrewmeyer]

As it happens, my current roadmap project is (very roughly put) "fix leaky stuff" so it fits quite nicely!