Segfault in atomdict setdefault and DefaultAtomDict __missing__ with key coercion #260

AtomDict_setdefault (atomdict.cpp:166) and DefaultAtomDict_missing (atomdict.cpp:311) re-lookup a value using the original key after AtomDict_ass_subscript stored it under a validated (possibly coerced) key. If the key validator coerces the key (e.g., Coerced(int) turns '42' into 42), the re-lookup with the original key fails. PyDict_GetItem returns NULL, which is passed to cppy::incref(NULL) — segfault.

Git history shows the pattern was introduced in commit 57957af (setdefault, March 2023) and copy-paste propagated to __missing__ in commit 0ff1ba6 (April 2023), including the identical comment: "Get the dictionary from the dict itself in case it was coerced.".

Reproducer 1 — setdefault:

import gc; gc.disable()
import os
from atom.api import Atom, Dict, Coerced

class MyAtom(Atom):
    d = Dict(key=Coerced(int))

a = MyAtom()
a.d[42] = "direct int"
a.d.setdefault("42", "coerced default")  # Segmentation fault
os._exit(0)

Reproducer 2 — missing:

import gc; gc.disable()
import os
from atom.api import Atom, Coerced
from atom.dict import DefaultDict

class MyAtom(Atom):
    d = DefaultDict(key=Coerced(int), missing=lambda: "generated")

a = MyAtom()
a.d["42"]  # Segmentation fault
os._exit(0)

Fix: the re-lookup should use the validated key. Since the validated key is not currently saved, the function needs restructuring. One approach for setdefault:

// Save the return of AtomDict_ass_subscript or use the validated key
cppy::ptr key_ptr( cppy::incref( key ) );
if( should_validate( self ) )
{
    key_ptr = validate_key( self, key_ptr.get() );
    if( !key_ptr )
        return 0;
}
// ... store with key_ptr, then re-lookup with key_ptr
return cppy::incref( PyDict_GetItem( pyobject_cast( self ), key_ptr.get() ) );

Found by cext-review-toolkit.