unix-ffi/re: Fix the PCRE2 memory leaks. - #1153
Draft
klukonin wants to merge 4 commits into
Draft
Conversation
Every call to search() allocated a match data block with pcre2_match_data_create_from_pattern() and never freed it again, leaking a few kilobytes per call, on the no-match path as well. Free it once the offsets have been copied out of it. The module level functions compile a pattern that the caller never gets to see, and that was leaked as well. Free it when the call is done; the match object that is returned does not refer to it. Note that a pattern returned by re.compile() still has to be kept alive by the caller and cannot be released automatically, because MicroPython does not run __del__ on instances of Python classes. Signed-off-by: Kirill Lukonin (Evil Wireless Man) <klukonin@gmail.com>
The error code and the error offset were passed as bytes(4). Such objects are immutable, and the error offset is a PCRE2_SIZE, which is 8 bytes on a 64-bit target, so a failing compile wrote 4 bytes past the end of the buffer. Use writable arrays of the right size instead, and report the values in the assertion. Signed-off-by: Kirill Lukonin (Evil Wireless Man) <klukonin@gmail.com>
The test measures the resident set size around a few thousand calls and fails if it keeps growing. It covers every entry point that makes PCRE2 allocate: matching with a compiled pattern, the module level functions, and compiling itself, including a pattern that fails to compile. Without the preceding fixes it reports between 4.6 and 17.8 kilobytes of growth per call, depending on the entry point. Signed-off-by: Kirill Lukonin (Evil Wireless Man) <klukonin@gmail.com>
Every call to re.search(), and to the functions next to it, compiled the pattern it was given. Keep the compiled patterns in a small cache instead, the way CPython does, so that using the same pattern again does not compile it a second time. compile() returns the cached pattern as well, so re.compile(p) is re.compile(p), as it is in CPython. Matching against a repeated pattern gets about twice as fast, compiling one about eight times. Because MicroPython cannot release a compiled pattern by itself, the cache also decides what is kept: a cached pattern stays for the lifetime of the program, and a pattern that this module compiled for its own use is freed again afterwards. The cache owns what it holds and never evicts it. A pattern that is still in use, by the caller or by a call further up the stack, must not be freed underneath it, which a replacement callback passed to sub() can otherwise trigger. The cache is bounded instead: once it is full, further patterns are compiled and, where this module owns them, freed again after use. Signed-off-by: Kirill Lukonin (Evil Wireless Man) <klukonin@gmail.com>
agatti
reviewed
Sep 3, 2026
| try: | ||
| return r.search(string) | ||
| finally: | ||
| if owned: |
Contributor
There was a problem hiding this comment.
If you put the owned check inside _free then it will probably be smaller once byte-compiled, so it won't have to be repeated multiple times.
| def _compile(pattern, flags): | ||
| # These are output arguments and must be writable and of the size that | ||
| # pcre2_compile() writes: int for the error code, PCRE2_SIZE for the offset. | ||
| errcode = array.array("i", [0]) |
Contributor
There was a problem hiding this comment.
This is actually the same as doing bytes(4). Check mp_binary_get_size in py/binary.c.
TL;DR: maybe the smallest fix is to update erroffset size from 4 to 8 and use int.from_bytes to perform the bytes->int conversion. Probably speed is the least of your worries for this function.
An array constructor takes 11 bytes (ignoring the extra entry in the string pool for the format string as that's already brought in elsewhere):
11:02 LOAD_NAME array
14:02 LOAD_METHOD array
10:03 LOAD_CONST_STRING i
80 LOAD_CONST_SMALL_INT 0
2b:01 BUILD_LIST 1
36:02 CALL_METHOD 2
whilst creating a bytes object just 5:
11:03 LOAD_NAME bytes
84 LOAD_CONST_SMALL_INT 4
34:01 CALL_FUNCTION 1
int.from_bytes is should probably suffice in this case to do the conversion, as this is only for printing (guaranteed?) positive numbers with a fixed upper size.
Shortening the message may also help here: how about "compile error %d at %d"?
| self.key = None # set while this pattern is held by the cache | ||
|
|
||
| def _free(self): | ||
| # MicroPython does not run __del__ on instances of Python classes, so |
Contributor
There was a problem hiding this comment.
You may want to see if weakref can help here, added in 1.27.0.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
unix-ffi/renever frees anything it gets from PCRE2. Every match leaks thematch data block, and every compiled pattern leaks as well, so a program or module (such as json) that
uses re module in a loop grows without bound.
Measured on a
ports/unixbuild (1.30.0-preview) against libpcre2-8, residentset size around 2x5000 calls:
Pattern.search(), match / no matchPattern.match()/sub()/split()/findall()re.search()/re.match()re.sub()/re.split()/re.findall()re.compile(), same patternre.compile(), distinct patterns70k operations grew the process from 4 MB to 367 MB before.
With the fixes it is flat now.