Skip to content

SortedMap_new: 4 bugs — double-incref leak, PyDict_Items leak, self leak on error, unchecked PySequence_GetItem segfault #259

Description

@devdanzin

SortedMap_new (sortedmap.cpp:284-331) has four interrelated bugs:

1. Double reference leak from PySequence_GetItem (line 325-326)

PySequence_GetItem returns a new reference. These raw PyObject* are passed to setitem() which calls MapItem(key, value) constructor (line 36) that does cppy::incref(key) and cppy::incref(value) — a second incref. Every key and value stored via this path leaks one reference.

Reproducer:

import gc; gc.disable()
import os
from atom.datastructures.sortedmap import sortedmap

class Trackable:
    live = 0
    def __init__(self, val):
        Trackable.live += 1; self.val = val
    def __del__(self):
        Trackable.live -= 1
    def __lt__(self, other): return self.val < other.val
    def __eq__(self, other): return self.val == other.val

pairs = [(Trackable(i), Trackable(i*10)) for i in range(50)]
m = sortedmap(pairs)
del pairs, m
print(f"After del: {Trackable.live} live (should be 0)")
# After del: 100 live (should be 0) — all 100 objects leaked
os._exit(0)

Fix: use cppy::ptr to capture the new references and pass via the MapItem(cppy::ptr&, cppy::ptr&) overload (line 38):

cppy::ptr key( PySequence_GetItem( item.get(), 0 ) );
cppy::ptr val( PySequence_GetItem( item.get(), 1 ) );
if( !key || !val ) { Py_DECREF(self); return 0; }
cself->setitem( key.get(), val.get() );

2. Unchecked PySequence_GetItem — segfault (same lines)

PySequence_GetItem can return NULL if __getitem__ raises. The NULL is passed directly to setitem()cppy::incref(NULL) → segfault.

Reproducer:

import gc; gc.disable()
import os
from atom.datastructures.sortedmap import sortedmap

class BrokenPair:
    def __len__(self): return 2
    def __getitem__(self, idx):
        if idx == 0: return "key"
        raise RuntimeError("getitem bomb")

sortedmap([BrokenPair()])  # Segmentation fault
os._exit(0)

3. PyDict_Items return value leaked (line 304)

PyDict_Items(map) returns a new reference. It is passed directly to PyObject_GetIter() without being stored — leaked every time a SortedMap is constructed from a dict.

Fix: wrap in cppy::ptr:

cppy::ptr items( PyDict_Items( map ) );
if( !items ) { Py_DECREF(self); return 0; }
seq = PyObject_GetIter( items.get() );

4. self leaked on all error paths (lines 306, 313, 323)

PyType_GenericNew returns a new reference stored in raw PyObject* self. Every early return leaks it.

Fix: add Py_DECREF(self) before each error return, or wrap self in a cppy::ptr.

Found by cext-review-toolkit.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions