angr / angr/angr-management

Instance.__init__ starts a logging QueueListener that is never stopped

オープン
#1,730 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
1.2k
フォーク
131
平均マージ
1日 11時間
マージ済み PR(30日)
14

説明

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

### Description

Every `Instance` starts a logging `QueueListener` and never stops one.
`Instance.__init__` calls `initialize(self)` unconditionally
(`angrmanagement/data/instance.py:96`), and `initialize`
(`angrmanagement/data/log.py:91-102`) is:

```python
def initialize(level=logging.NOTSET) -> None:
queue = Queue()
Initializer.get().register(install_queue_handler, queue)
install_queue_handler(queue)
listener = QueueListener(queue, LogDumpHandler(level))
atexit.register(listener.stop)
listener.start()
```

`listener` is a local. The only reference that outlives the call is the one
`atexit` holds, so the listener, its monitor thread and its
`multiprocessing.Queue` live until the process exits. `grep -n "atexit" ` over
`angrmanagement/data/log.py` returns those two lines and nothing else, and
`MainWindow.closeEvent` stops plugins, jobs and the MCP server but nothing here.

The guard on line 87 is on the *root logger handler*, not on the listener, so
it does not prevent any of this — it only ensures that the first queue is the
one wired to `logging.root`. Listeners 2..N therefore sit forever on queues
that nothing feeds, and the `LogDumpHandler` for every `Instance` after the
first never receives a record.

Two things fall out of the same code. `initialize(level=logging.NOTSET)` is
handed an `Instance`, which binds positionally to `LogDumpHandler.__init__`'s
`instance` parameter (`log.py:64`) — the parameter name and its implied type
are wrong, and the level can never be set. And `LogDumpHandler` keeps
`self.instance` (`log.py:66`) and calls `self.instance.log.am_event(...)` on
every record (`log.py:71`), so `atexit` transitively retains every `Instance`
ever built, together with whatever that `Instance` reaches — its `Project`, its
knowledge base, and through `Instance.log`'s subscribers
(`ui/widgets/qlog_widget.py:174`, unsubscribed only in that widget's own
`closeEvent`) the windows and views as well. In a long GUI session or a test
worker that is the larger cost; the threads are just the visible part.

### Steps to reproduce the bug

The listener half is stdlib-only. This is `initialize()` transcribed with angr
and Qt removed, so it measures the shape rather than the app:

```python
def initialize(instance):
queue = Queue()
registry.append(queue) # stands in for Initializer.register
install_queue_handler(queue)
listener = QueueListener(queue, Dump(instance))
atexit.register(listener.stop)
listener.start()
```

```
baseline: threads=1 monitor=0 fds=4
after 1 initialize(): threads= 2 monitor= 1 fds= 6
after 10 initialize(): threads= 11 monitor= 10 fds= 24
after 25 initialize(): threads= 26 monitor= 25 fds= 54
after 50 initialize(): threads= 51 monitor= 50 fds=104
root queue handlers: 1
after gc: monitor=50 fds=104
```

One `QueueListener._monitor` thread and two file descriptors per call, not
reclaimed by `gc.collect()`, with one root handler however many times it runs.

In the test suite that is once per test method. `tests/common.py`'s
`AngrManagementTestCase.setUp` builds `MainWindow(show=False)`, which builds a
`Workspace`, which builds an `Instance`; `tearDown` does `self.main.close()`
and `del self.main`, neither of which reaches the listener. Counting `def test`
in the files that use those base classes gives **602**. CI runs
`uv run pytest -vv -n auto tests` (`.github/workflows/ci.yml:65`) with no
`--forked`, so each worker accumulates its share for the length of the run.

### What this is not

**This is not offered as the cause of #1729**, and it should not be folded into
it. That issue has a core-dump-level root cause; this is an unproven
contributor at best, and attaching it would muddy a clean report. Filing it
here on leak grounds alone.

One number is worth correcting before it travels, because it is easy to
transpose. A dump showing **19 of 24 threads** in
`logging/handlers.py:1593 in _monitor` comes from an `angr/mono` CI job, not
from #1648. #1648's own dump has **four** thread headers and **one** monitor
thread, which I re-counted from the issue body. So the leak had accumulated a
single `Instance`'s worth of state at that crash — whatever killed it did not
need many threads.

### Prior art

- #786 (merged 2022-11-21) introduced `log.py` in this shape.
- #797 "Exception on close" is this design's shutdown path failing: an
`EOFError` out of `QueueListener._monitor` when the main window closes,
reported four days after #786 landed.

Nothing in the tracker mentions the leak itself.

### Suggested shape

Not a `_initialized` flag: that stops the thread growth but freezes the present
accident, where only the first `Instance` ever receives records, and it still
retains that first one forever.

- Keep the queue and the listener as module-level singletons in `log.py`,
created on first use.
- `initialize(instance)` adds that instance's `LogDumpHandler` to the shared
listener; a new `shutdown(instance)` removes it, and stops the listener and
drops the root handler when the last one goes.
- Give `Instance` a `close()` that calls it, from `MainWindow.closeEvent` and
from the precise-diffing plugin where it drops its second instance.
- Fix `initialize`'s `level` parameter, which is really `instance`.

Worth knowing before changing it: with a shared listener every registered
handler starts receiving records, including a diffing plugin's second instance
that receives none today. That is arguably correct and is still a behaviour
change.

A test that pins it: build five `Instance`s and assert the `_monitor` thread
count rises by at most one and `/proc/self/fd` by at most two, and that an
`Instance` is weakref-collectable after `close()`. The second assertion is the
one that pins the retention.

### Environment

Read at `aa843e5c10645ba361620768d59642a460805e80`. The measurement above is
stdlib-only under CPython 3.12 and involves no angr, Qt or GUI.

session: sharpen

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。