fix(system): Handle psutil.AccessDenied in get_memory_info#2078
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens get_memory_info() (used for autoscaler CPU/memory snapshots) against psutil.AccessDenied in restricted environments, preventing the periodic system-info task from crashing and silently stopping.
Changes:
- Catch
psutil.AccessDeniedwhen reading the current process’s memory via PSS and fall back to RSS. - Suppress
psutil.AccessDeniedwhen aggregating child process memory usage. - Add unit tests covering both the child-skip and current-process RSS fallback behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/crawlee/_utils/system.py | Adds AccessDenied handling/fallbacks in memory snapshot collection. |
| tests/unit/_utils/test_system.py | Adds regression tests for AccessDenied scenarios in get_memory_info(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
On Linux, get_memory_info reads a process's PSS via memory_full_info, which psutil documents may require elevated privileges. In restricted environments (hardened containers with hidepid/restricted /proc, or an unreadable child subprocess) this raises psutil.AccessDenied, which is not a subclass of psutil.NoSuchProcess and so was not caught by the existing suppress around the child loop; the current-process read was unguarded entirely. The exception propagated out of the recurring system-info task, which has no error handling, silently stopping the autoscaler's CPU/memory snapshots. Suppress AccessDenied alongside NoSuchProcess when summing child memory, and fall back to RSS for the current process when PSS is denied. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
b3464f1 to
cd699de
Compare
|
pushed a revision addressing the latest review and ci feedback. |
|
Same as here #2074 (comment)
|
vdusek
left a comment
There was a problem hiding this comment.
Thanks for the revision. Both of my earlier comments are correctly addressed - the logger.debug for the PSS->RSS fallback, and the child RSS fallback so an unreadable child is no longer dropped.
However, the change made for the Copilot comment about guarding children() introduces a regression: wrapping the entire loop in suppress means one child dying mid-iteration now aborts the loop and drops all remaining children. I verified this locally - a repro with a dead first child and a live second child worth 40 B yields 140 B on master and 100 B on this branch. CI does not catch it because that path is untested.
Details inline.
✍️ Drafted by Claude Code
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
|
@vdusek addressed all new threads, including the child-exit regression test, rebased, and re-requesting review. thank you. |
get_memory_info()insrc/crawlee/_utils/system.pycan crash withpsutil.AccessDeniedin restricted environments, which silently stops the autoscaler's CPU/memory snapshots._get_used_memoryreads a process's PSS viaprocess.memory_full_info()(/proc/<pid>/smaps). psutil documents thatmemory_full_info/ PSS may require elevated privileges, so in a hardened container (hidepid, restricted/proc, seccomp) or for an unreadable child subprocess it raisespsutil.AccessDenied.psutil.AccessDeniedis not a subclass ofpsutil.NoSuchProcess(MRO:AccessDenied -> Error -> Exception), so the existingwith suppress(psutil.NoSuchProcess)around the child loop did not catch it, and the current-process read at the top of the function was not guarded at all.SYSTEM_INFOtask (RecurringTask._wrapperhas no error handling), so the recurring task stops and the autoscaler quietly stops receiving snapshots.Fix, kept minimal:
A process can always read its own RSS, so the current-process fallback keeps a valid (slightly higher, RSS vs PSS) estimate instead of aborting the whole snapshot; an unreadable child is skipped just like a dead one.
Issues
get_memory_infofor cross-platform robustness (in the same spirit as the earlier PSS-on-Linux and macOS memory-estimation fixes to this file). Happy to open a tracking issue with the repro first if you'd prefer.get_memory_infodoes not raiseAccessDeniedin the first place (no file overlap; the two are complementary).Testing
tests/unit/_utils/test_system.py:test_get_memory_info_skips_children_with_access_denied: a child that raisesAccessDeniedis skipped rather than aborting the snapshot.test_get_memory_info_falls_back_to_rss_when_current_process_access_denied: PSS denial for the current process falls back to RSS.src/crawlee/_utils/system.py(keeping the new tests) makes both tests fail withpsutil.AccessDeniedpropagating out ofget_memory_info(); they pass with the fix.uv run pytest tests/unit/_utils/test_system.py-> 4 passed, 1 skipped (the skip is the Linux-only shared-memory test).uv run pytest tests/unit/events/ tests/unit/_autoscaling/-> green.uv run poe lint(ruff format + check) anduv run ty checkon the changed files both pass.Checklist