-
-
Notifications
You must be signed in to change notification settings - Fork 34k
gh-144281: Fix crash on memoryview slice assignment with shared memory #144284
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
gh-144281: Fix crash on memoryview slice assignment with shared memory #144284
Conversation
20c6f0b to
4cfbb0a
Compare
|
|
||
| CHECK_RELEASED_INT(self); | ||
|
|
||
| if (view->buf == NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear this is a necessary or sufficient fix without a valid specific trigger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve updated the test that should also make the need for the view->buf NULL check clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is moot since this has been closed, but I'm afraid this still wouldn't have been useful. Your fix is unrelated to the test; it passes with or without it. It is good practice to always run your test scripts against affected versions, to confirm it reproduces the bug. Without that you don't know the test works.
Also note the bug in the OP is a SIGBUS, a fatal signal. It is completely expected that invalid operations like this raise exceptions, but it shouldn't be possible to trigger a fatal signal via the public API-ish. Terms and conditions apply. Offer void when ctypes or mmap or gc or various other low-level power tools.
|
I don't think this can be right. Partly because if it is you'd need to guard every use of |
|
If this is a legitimate issue, I believe this should be fixed as part of #143324. I will take care of that once I have time as well. More generally, I would prefer that you avoid addressing crash issues because many of your PRs are eventually closed and the issue is usually non-trivial so it is better to have a more seasoned contributor for those issues. |
Fix a possible interpreter crash when assigning to a memoryview slice backed by shared memory by validating the underlying buffer before writing. A regression test and NEWS entry are included.