-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
BufferError with memory.release() #79867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
See there: I did the first changeset after seeing some strange exception popping up which it was handling another exception - which I assumed was related to memoryview.release not being called in the original code. So it was clear to me, that we should use the CM there. So I added that (first changeset) and that made the code always fail (see first travis-ci link). Then removed the CM again and replaced it with functionally equivalent try/finally (second changeset) - that worked. So, the question is whether there is some issue in CPython's memoryview contextmanager code that make it fail in such a strange way. |
Thanks for the report. Can you please add a simple reproducer in Python with what the test is trying to do without dependencies from the project? perhaps with a sample of relevant files used by the test in Travis. |
We use "crash" for segmentation fault, but this appears to be a regular traceback that includes BufferError. The BufferError message appears to be from mmapmodule.c. |
The behavior seems to be correct to me: If there are exports, the memoryview cannot be released. The application needs to ensure that release() is not called when |
working, but potentially problematic because .release is not always called (e.g. in case of an exception). |
with context manager, does not work. |
with try/finally: works. |
hmm, issue tracker does not make it easy to see which file was uploaded for which comment, so: a: original code, works, potentially problematic exception handling (memoryview not always being released) b: using the memoryview context manager, fails with BufferError c: using try/finally to make sure memoryview is released: works |
I see 2 issues here:
|
Well, the problem in b) is that data[:2] creates a new memoryview,
So memoryview.__exit__() decrements on export, but the second one This actually works as expected because the ManagedBufferObject One way of doing so would be this: with open(fn, 'rb') as fd:
with mmap.mmap(fd.fileno(), 0, access=mmap.ACCESS_READ) as mm:
with memoryview(mm) as data:
with data[:2] as theslice:
print(theslice) |
s/on export/one export/ |
Or, obviously: with open(fn, 'rb') as fd:
with mmap.mmap(fd.fileno(), 0, access=mmap.ACCESS_READ) as mm:
with memoryview(mm)[:2] as data:
print(data) |
Doesn't this rely on the immediate finalization of the unreferenced memoryview instance that creates the slice? It would be nice if memoryview supported alternate constructors memoryview(obj, stop) and memoryview(obj, start, stop[, step]). |
https://github.com/borgbackup/borg/pull/4247/files this is the current code i have for this (it is not as simple there as in the minimal code samples i attached here). i meanwhile think i can not use a contextmanager there. do you think this is as good as it gets for this kind of code? if you all think this is expected behaviour for the python contextmanagers and can't be done better, this issue can be closed. |
Eryk Sun: Yes, the behavior is technically not guaranteed. I'm not sure about Thomas Waldmann:
I guess so, there's a lot going on in that code fragment. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: