Skip to content

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

@devdanzin

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.

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