gh-127076: Ignore memory mmap in FileIO testing #127088
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`mmap`, `munmap`, and `mprotect` are used by CPython for memory management, which may occur in the middle of the FileIO tests. The system calls can also be used with files, so `strace` includes them in its `%file` and `%desc` filters. Filter out the `mmap` system calls related to memory allocation for the file tests. Currently FileIO doesn't do `mmap` at all, so didn't add code to track from `mmap` through `munmap` since it wouldn't be used. For now if an `mmap` on a fd happens, the call will be included (which may cause test to fail), and at that time support for tracking the address throug `munmap` could be added.
cmaloney
changed the title
mgorny
reviewed
Nov 21, 2024
|
Validated by modifying # "open, read, close" file using different common patterns.
check_readall(
"open builtin with default options",
f"""
f = open('{TESTFN}')
a = "123" * 100000
f.read()
f.close()
"""
) |
mgorny
approved these changes
Nov 22, 2024
vstinner
reviewed
Nov 22, 2024
Lib/test/support/strace_helper.py
Outdated
| # mmap can operate on a fd or "MAP_ANON" which gives a block of memory. | ||
| # Ignore the "MAP_ANON" ones. | ||
| if call.syscall == "mmap" and "MAP_ANON" in call.args[3]: | ||
| return False |
There was a problem hiding this comment.
Would it be possible to return True if the syscall should be filtered, and False otherwise? In short, replace if _filter(call) with if not _filter(call).
Lib/test/support/strace_helper.py
Outdated
| of memory. Use this function to filter out the memory related calls from | ||
| other calls.""" | ||
|
|
||
| def _filter(call): |
There was a problem hiding this comment.
Can you move this function at the module level? Maybe rename it to _filter_memory_call().
There was a problem hiding this comment.
Can move, my thought initially was "Track the address mmap returns through munmap" and the set/dictionary of "known addresses" would be in the outer scope, but I don't think mmap in io is likely (there's the mmap module if it's better for a particular use case)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
mmap,munmap, andmprotectare used by CPython for memory management which may occur in the middle of the FileIO tests. The system calls can also be used with files, sostraceincludes them in its%fileand%descfilters.Filter out the
mmapsystem calls related to memory allocation for the file tests. Currently FileIO doesn't dommapat all, so didn't add code to track frommmapthroughmunmapsince it wouldn't be used. For now if anmmapon a fd happens, the call will be included (which may cause tests to fail), and at that time support for tracking the address throughmunmapcould be added.cc: @mgorny this should fix the glibc and musl Gentoo general failures outside sandbox
cc: @brandtbucher This re-enables for PYTHON_JIT, and filters out the
mmapcalls used for memory management