Skip to content

Commit

Permalink
Fix leak when importing value
Browse files Browse the repository at this point in the history
Summary:
When importing a lazy value if we can trigger this lookup while already doing an import. One chain of events is:
`globals().items()` -> forced import of all values -> (import of package, not defined, does `hasattr` to see if it's there -> triggers import) x 10.

As we unwind through the chain of getattrs we continuously leak a reference to the value as the inner most one changed it. So we need to be more aggressive about what we do when checking if the dictionary changed. This causes us to retry the lookup if keys/kind/or the current key have changed. If only the value has changed we return the new value instead of the one we just imported.

Reviewed By: Kronuz

Differential Revision: D68735993

fbshipit-source-id: 03365011e0eccb2297bfb10886be9a013e331140
  • Loading branch information
DinoV authored and facebook-github-bot committed Jan 29, 2025
1 parent 3f27e04 commit 784d911
Showing 1 changed file with 21 additions and 10 deletions.
31 changes: 21 additions & 10 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1182,16 +1182,27 @@ _Py_dict_lookup(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **valu
}
return DKIX_EMPTY;
}
/* If the dict hasn't mutated, update resolved_value */
if (dk == mp->ma_keys && kind == dk->dk_kind) {
if (kind != DICT_KEYS_GENERAL
? DK_UNICODE_ENTRIES(dk)[ix].me_key == startkey
: DK_ENTRIES(dk)[ix].me_key == startkey) {
if (*value_ptr == value) {
Py_DECREF(*value_ptr);
*value_ptr = resolved_value;
}
}
/* If the dict hasn't mutated, update resolved_value, otherwise
* retry the lookup */
if (dk != mp->ma_keys || kind != dk->dk_kind) {
Py_DECREF(resolved_value);
goto start;
}

PyObject *key = kind != DICT_KEYS_GENERAL
? DK_UNICODE_ENTRIES(dk)[ix].me_key : DK_ENTRIES(dk)[ix].me_key;
if (key != startkey) {
Py_DECREF(resolved_value);
goto start;
}

if (*value_ptr == value) {
Py_DECREF(*value_ptr);
*value_ptr = resolved_value;
} else {
// Just the value was replaced, so return the new value.
Py_DECREF(resolved_value);
resolved_value = *value_ptr;
}
value = resolved_value;
}
Expand Down

0 comments on commit 784d911

Please sign in to comment.