Skip to content

Fix file descriptor leak when building global_event_df - #1245

Merged
jhnwu3 merged 1 commit into
sunlabuiuc:masterfrom
solarsys:fix/dask-dashboard-fd-leak
Sep 27, 2026
Merged

jhnwu3 merged 1 commit into
sunlabuiuc:masterfrom
solarsys:fix/dask-dashboard-fd-leak

Conversation

@solarsys

Copy link
Copy Markdown
Collaborator

Problem

Each BaseDataset._event_transform starts a dask LocalCluster. On close, the cluster's bokeh dashboard fails to stop:

RuntimeError: Cannot synchronously wait on a running event loop; use 'await stop_async()'

Each dataset build therefore leaks 3 file descriptors. A process that builds many datasets (the core test suite, or a benchmark_workers_n.py sweep) exhausts macOS's default ulimit -n of 256 and fails partway through with:

OSError: [Errno 24] Too many open files

Linux CI doesn't hit this because its default limit is much higher. Every macOS contributor running the suite locally does.

Fix

Pass dashboard_address=None to LocalCluster in _event_transform. Nothing in PyHealth uses the dashboard. This also removes the repeated Port 8787 is already in use warnings.

Changes

  • pyhealth/datasets/base_dataset.py: disable the dask dashboard (one line plus a comment).
  • tests/core/test_base_dataset.py: regression test that builds global_event_df 4 times and asserts the open-fd count stays flat. Skipped on Windows.
  • docs/install.rst: macOS entry under Platform-Specific Notes, with the ulimit -n workaround for 2.0.2 and earlier.
  • examples/benchmark_perf/benchmark_workers_n.py: records open_fds after each run, so the sweep would surface a regression like this.

Verification (macOS, Python 3.13)

  • Open fds after consecutive builds: before the fix they grow 9 → 12 → 15 → 18 → 21 → 24; after it they stay at 6.
  • The new test fails without the fix (12 file descriptors leaked across 4 dataset builds) and passes with it.
  • Full core suite with ulimit -n 256: Ran 1328 tests … OK (skipped=76), with 0 "Too many open files" errors. Before the fix the same run failed.
  • tools/check_pr_rules.py --base master --head HEAD: all rules pass.
  • The benchmark example is only compile-checked because it needs full MIMIC-IV.

🤖 Generated with Claude Code

Each BaseDataset._event_transform starts a dask LocalCluster whose bokeh
dashboard fails to stop on close ("Cannot synchronously wait on a running
event loop"), leaking 3 file descriptors per dataset build. Processes that
build many datasets (the core test suite, benchmark sweeps) exhaust the
macOS default `ulimit -n` of 256 and fail with "OSError: [Errno 24] Too
many open files". Nothing in PyHealth uses the dashboard, so disable it.

- base_dataset: pass dashboard_address=None to LocalCluster
- tests: regression test that fd count stays flat across 4 builds
  (fails with 12 leaked fds without the fix)
- docs/install.rst: macOS note with the ulimit workaround for <= 2.0.2
- examples/benchmark_perf/benchmark_workers_n.py: report open_fds per run

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@solarsys
solarsys requested a review from jhnwu3 September 27, 2026 15:54

@jhnwu3 jhnwu3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will dig deeper into this later. If this fixes the issue with anything else and unblocks you, that's an interesting problem I hadn't considered at the extreme end.

@jhnwu3
jhnwu3 merged commit d391dde into sunlabuiuc:master Sep 27, 2026
2 checks passed
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.

2 participants