Repository navigation
Hard crash when deleting from Dictionary #2530
Description
Activity
Thank you for the reproducible example, I will look into it. If you want to have a go at it, you are more than welcome though.
Side note: I have checked that this is not a regression in today's release, the same error happens on all 3.* versions.
My first guess is that we simply don't implement the deletion case. For
__delitem__, Python passes anullto thevalueparameter ofmp_ass_subscript.mp_ass_subscriptis implemented via Indexers which does not have an equivalent to__delitem__.So, we have the following options:
- Throw an exception (that can be caught), I think this is a good first step
- For
ICollection,IDictionary,IList, and classes that implement these, special-case scalar deletions to use the respective remove functions
Hm, this actually should work due to
Maybe it is not enabled by default? It might not be enabled due to scenarios where you don't want these methods to appear in any kind of reflection (from Python).
And it would work, except that the
Dictionaryclass provides its own implementation of__delitem__that overrides the one inMutableMappingMixin:>>> import pythonnet; pythonnet.load('coreclr'); import clr >>> import System >>> DictType = System.Collections.Generic.Dictionary[System.String, System.Int32] >>> d = DictType() >>> d['a'] = 1 >>> d['b'] = 2 >>> d['c'] = 3 >>> for cls in DictType.mro(): # print the classes that implement __delitem__ ... if hasattr(cls, '__delitem__'): ... print('@@@', cls) @@@ <class 'System.Collections.Generic.Dictionary[String,Int32]'> @@@ <class 'clr._extras.collections.MutableMappingMixin'> @@@ <class 'collections.abc.MutableMapping'> >>> MutableMappingMixin = DictType.mro()[2] >>> MutableMappingMixin <class 'clr._extras.collections.MutableMappingMixin'> >>> dict(d) {'a': 1, 'b': 2, 'c': 3} >>> MutableMappingMixin.__delitem__(d, 'c') # Calling directly using base class, works OK >>> dict(d) {'a': 1, 'b': 2} >>> super(DictType, d).__delitem__('b') # Calling on superclass also works OK >>> dict(d) {'a': 1} >>> DictType.__delitem__(d, 'a') # Equivalent to d.__delitem('a') and del d['a'], crashes Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object. at Python.Runtime.BorrowedReference.DangerousGetAddress() in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/BorrowedReference.cs:line 18 at Python.Runtime.NewReference..ctor(BorrowedReference reference, Boolean canBeNull) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/NewReference.cs:line 20 at Python.Runtime.Runtime.PyTuple_SetItem(BorrowedReference pointer, IntPtr index, BorrowedReference value) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Runtime.cs:line 1482 at Python.Runtime.ClassBase.mp_ass_subscript_impl(BorrowedReference ob, BorrowedReference idx, BorrowedReference v) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Types/ClassBase.cs:line 498
Well, the direct implementation always wins, and Python, counts
mp_ass_subscriptas an implementation for__setitem__and__delitem__. We could go through the__mro__to find the mixin, but at that point it might make more sense to just extend themp_ass_subscriptimplementation to handle the deletion itself.Reacted by Diego Baldassar- added 2 commits that reference this issue
on Apr 11, 2025 - added a commit that references this issue
on Apr 11, 2025
Environment
Details
Remove an element from a .NET Dictionary using the del operator (
__delitem__).If you can provide a Minimal, Complete, and Verifiable example this will help us understand the issue.
I reduced the problem to the following example, simply run from the command line:
The program crashed with the following output:
Notice that
__delitem__is also implemented by theMutableMappingMixinclass here, which simply calls theRemove()method, butDictionaryhas its own implementation (that I can't find), which takes precedence.The former would have worked correctly (tested), as
Remove()works as usual.