Skip to content

Conversation

@t-vi
Copy link
Collaborator

@t-vi t-vi commented Oct 16, 2018

The mapping protocol stipulates that when __delitem__ is called, this is passed to __setitem__ (well, the same function in the C extension interface) with NULL data.

PyTorch master crashes in this situation, with this patch, it does not anymore.

Test code (careful, sefaults your interpreter):

import torch
a = torch.randn(5)
del a[2]

The mapping protocol stipulates that when __delitem__
is called, this is passed to "__setitem__" (well, the same
function in the C extension interface) with NULL data.

https://docs.python.org/3/c-api/typeobj.html#c.PyMappingMethods.mp_ass_subscript

PyTorch master crashes in this situation, with the patch, it does
not anymore.
Copy link
Member

@colesbury colesbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

colesbury has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants