Skip to content

Fix: del obj[key] on a reflected indexer crashed the process - #149

Open
jhonabreul wants to merge 1 commit into
QuantConnect:masterfrom
jhonabreul:bug-del-item-crash
Open

Fix: del obj[key] on a reflected indexer crashed the process#149
jhonabreul wants to merge 1 commit into
QuantConnect:masterfrom
jhonabreul:bug-del-item-crash

Conversation

@jhonabreul

@jhonabreul jhonabreul commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • del obj[key] on any reflected type with a settable indexer crashed the process.
  • CPython uses the same mp_ass_subscript slot for assignment and deletion, with a null value for del.
  • mp_ass_subscript_impl never checked for the null and forwarded it into PyTuple_SetItem, which threw across the native boundary.
  • ArrayObject.mp_ass_subscript had the same hole and died with an access violation.
  • Official pythonnet fixed this in __delitem__ for IList<T> and IDictionary<K,V> pythonnet/pythonnet#2533; this fork predates it.

Changes

  • ClassBase: check v.IsNull first and route to the new DeleteItemImpl.
  • DeleteItemImpl: TypeError when the type has no deleter or the key is a tuple; otherwise call the deleter through the binder; false from Remove becomes KeyError.
  • Indexer: new DeleterBinder, resolved from IDictionary<K,V>.Remove(K), else IList<T>.RemoveAt(int). Uses the interface method, so explicit implementations like ConcurrentDictionary.Remove work.
  • ArrayObject: TypeError on a null value.
  • Tests: nine pytest cases, a throwing Remove fixture, and an embed test fixture.

Behaviour

Target Before After
plain settable indexer, tuple key, array crash TypeError
Dictionary / ConcurrentDictionary crash key removed, KeyError if missing
List<T> crash element removed, ArgumentOutOfRangeException if out of range
deleter throws n/a catchable exception, interpreter alive

Verification

  • pytest: 477 passed, 1 pre-existing environmental failure (test_explicit_assembly_load).
  • Embed tests (Debug): 3 passed. Each new test crashes on master.
  • Lean with the patched DLL: a Python algorithm doing del self.runtime_statistics[key] now completes; 324/324 Python regression algorithms and 16600/16600 Python unit tests pass.

Follow-up

Closes https://github.com/QuantConnect/PythonnetEnterprise/issues/167

CPython calls mp_ass_subscript with a null value for del, which mp_ass_subscript_impl forwarded into PyTuple_SetItem and threw across the native boundary. Handle deletion first: IDictionary<K,V>.Remove / IList<T>.RemoveAt through the binder (KeyError on a missing dictionary key), TypeError for every other type and for arrays.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant