Rather than open-code the Py_RTFLAGS_USE_MAIN_OBMALLOC flag test in multiple places, I think a helper function is better, something like:

#ifdef Py_TRACE_REFS// Return true if interpreter should have it's own refchain table.  Otherwise// the refchain from the main interpreter is shared.static inline boolhas_own_refchain(PyInterpreterState *interp){    return (_Py_IsMainInterpreter(interp) ||            !(interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC));}#endif

I was working on a similar patch but my approach was to copy the REFCHAIN pointer, e.g.

PyStatus_PyObject_InitState(PyInterpreterState *interp){#ifdef Py_TRACE_REFS    if (has_own_refchain(interp)) {        _Py_hashtable_allocator_t alloc = {            // Don't use default PyMem_Malloc() and PyMem_Free() which            // require the caller to hold the GIL.            .malloc = PyMem_RawMalloc,            .free = PyMem_RawFree,        };        REFCHAIN(interp) = _Py_hashtable_new_full(            _Py_hashtable_hash_ptr, _Py_hashtable_compare_direct,            NULL, NULL, &alloc);        if (REFCHAIN(interp) == NULL) {            return _PyStatus_NO_MEMORY();        }    }    else {        PyInterpreterState *main_interp = _PyInterpreterState_Main();        REFCHAIN(interp) = main_interp->object_state.refchain;    }#endif    return _PyStatus_OK();}void_PyObject_FiniState(PyInterpreterState *interp){#ifdef Py_TRACE_REFS    if (has_own_refchain(interp)) {        _Py_hashtable_destroy(REFCHAIN(interp));    }    REFCHAIN(interp) = NULL;#endif}

Is_Py_NormalizeImmortalReference still required now? I would think it can be removed. Overall I think this is a good approach.

I think a helper function is better

+1 I'll do that.

my approach was to copy the REFCHAIN pointer

I'm unclear on how that's different from what I've done.

Is _Py_NormalizeImmortalReference still required now? I would think it can be removed.

I left it for the sake of the sanity check. Given the lingering pain in this area, especially related to immortal objects, I figured it might not hurt to preserve such checks.

Overall I think this is a good approach.

Thanks! Sorry if we ended up duplicating effort. I should have checked with you first.

my approach was to copy the REFCHAIN pointer

I'm unclear on how that's different from what I've done.

I think it's a bit less code and would be a bit faster, since you can just useREFCHAIN() everywhere aside from the init/fini function. Since TraceRefs is has huge overhead anyhow I think it doesn't matter so it's just a code style decision. Your way works and you could argue that having the condition everytime you do something with the tracerefs is more clear.

Is _Py_NormalizeImmortalReference still required now? I would think it can be removed.

I left it for the sake of the sanity check. Given the lingering pain in this area, especially related to immortal objects, I figured it might not hurt to preserve such checks.

Hmm, I think it's a bit confusing to leave it since it really shouldn't be doing anything useful. Or, is there a reason we think it might do something? The second part of the function shouldn't do anything. IfPy_RTFLAGS_USE_MAIN_OBMALLOC is set then both interpreters are using the same refchain hash table and you are adding/removing to the same table.

Overall I think this is a good approach.

Thanks! Sorry if we ended up duplicating effort. I should have checked with you first.

Your code runs without crashing, which is a plus. ;-) I'm glad you made a PR.

Oh, I missed this part of your code change:

REFCHAIN(interp) = REFCHAIN(main_interp);

That explains why you're unclear how my code is different. But if you do this in the init, I don't think the check forPy_RTFLAGS_USE_MAIN_OBMALLOC is needed elsewhere, you can just useREFCHAIN(interp) when you need to add or remove objects to the hash table.

I don't think the check for Py_RTFLAGS_USE_MAIN_OBMALLOC is needed elsewhere, you can just use REFCHAIN(interp) when you need to add or remove objects to the hash table.

Good point. I'll remove the superfluous calls.

Also, you're right about_Py_NormalizeImmortalReference(). I'll drop it.

I realized the problem with dropping all the extrahas_own_refchain() sites. Here are the key steps of what we do currently:

1. Py_NewInterpreterFromConfig(): calls_PyInterpreterState_New()
2. _PyInterpreterState_New(): calls_PyObject_InitState()
3. _PyObject_InitState(): sets the interpreter's refchain depending oninterp->feature_flags
4. _PyInterpreterState_New(): calls other initializers that create objects, which add to the refchain
5. Py_NewInterpreterFromConfig(): setsinterp->feature_flags

At step (3) we rely on the feature flags being set, but we don't set them until step (5).

Fixing this is a bit messy and may be too much to backport, so I'd like to merge this fix with the varioushas_own_refchain() calls in place (sort of), and then tackle the rest in a separate PR. It's not that simple, but you'll see what I mean.

It wasn't as bad as I thought, so I believe I've got things in a good place here.

!buildbot AMD64 Arch Linux TraceRefs

🤖 New build scheduled with the buildbot fleet by@ericsnowcurrently for commit7ab086c 🤖

The command will test the builders whose names match following regular expression:AMD64 Arch Linux TraceRefs

The builders matched are:

• AMD64 Arch Linux TraceRefs PR

I think this change should have a What's New entry, which in turn needs documentation forsys.getobjects:ericsnowcurrently#39

I'm fine with that.

!buildbot AMD64 Arch Linux TraceRefs

🤖 New build scheduled with the buildbot fleet by@ericsnowcurrently for commit1360d86 🤖

The command will test the builders whose names match following regular expression:AMD64 Arch Linux TraceRefs

The builders matched are:

• AMD64 Arch Linux TraceRefs PR

One more doc fix in a place where I couldn't add a GitHub suggestion. I hope you don't mind me pushing this directly to the PR.

TheAMD64 Arch Linux TraceRefs PR buildbot run succeded:https://buildbot.python.org/#/builders/82/builds/1457

@nascheme, this should be backported to 3.13 (and 3.12), right?

For 3.12, this fix applies on top ofGH-125205, which is not yet merged. For 3.13,GH-125204 is also not yet merged.