gh-104252: Immortalize Py_EMPTY_KEYS

[ericsnowcurrently]

This was missed in gh-19474. It matters for with a per-interpreter GIL since PyDictKeysObject.dk_refcnt breaks isolation and leads to races.

cc @eduardo-elizondo

• Issue: Immortalize Py_EMPTY_KEYS #104252

[sunmy2019]

There are other places with direct changes to dk_refcnt. They are not guarded.

cpython/Objects/dictobject.c

Line 639 in 4729383

dk->dk_refcnt=1;

cpython/Objects/dictobject.c

Lines 822 to 828 in 4729383

	/* Since we copied the keys table we now have an extra reference
	in the system. Manually call increment _Py_RefTotal to signal that
	we have it now; calling dictkeys_incref would be an error as
	keys->dk_refcnt is already set to 1 (after memcpy). */
	#ifdefPy_REF_DEBUG
	_Py_IncRefTotal(_PyInterpreterState_GET());
	#endif

cpython/Objects/dictobject.c

Lines 1530 to 1538 in 4729383

	// We can not use free_keys_object here because key's reference
	// are moved already.
	#ifdefPy_REF_DEBUG
	_Py_DecRefTotal(_PyInterpreterState_GET());
	#endif
	if (oldkeys==Py_EMPTY_KEYS){
	oldkeys->dk_refcnt--;
	assert(oldkeys->dk_refcnt>0);
	}

[ericsnowcurrently]

@sunmy2019, thanks for pointing out the other uses of dk_refcnt. I'll fix that.

[done]

[methane]

Please remove these lines too:

cpython/Objects/dictobject.c

Line 849 in 58585a9

dictkeys_incref(Py_EMPTY_KEYS);

cpython/Objects/dictobject.c

Line 1347 in 58585a9

dictkeys_decref(interp, Py_EMPTY_KEYS);

And Move _Py_DecRefTotal() into if (oldkeys != Py_EMPTY_KEYS) block.

cpython/Objects/dictobject.c

Lines 1545 to 1548 in 58585a9

	#ifdefPy_REF_DEBUG
	_Py_DecRefTotal(_PyInterpreterState_GET());
	#endif
	if (oldkeys!=Py_EMPTY_KEYS){

[ericsnowcurrently]

Thanks for the feedback. I'll clean things up tomorrow.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @sunmy2019 for commit 5aabfd7 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

[sunmy2019]

LGTM! now it's much cleaner

[ericsnowcurrently]

Thanks for the reviews!

[eduardo-elizondo]

Nice fix! I just got around to looking at this PR, it was weird thinking about the PyDictKeysObject since they were not PyObjects, so I just let them be. That being said, this LGTM specially if we are trying to share as much state as possible.