gh-128058: Fix test_builtin ImmortalTests

[vstinner]

On 32-bit systems with Free Threading, _Py_IMMORTAL_INITIAL_REFCNT is defined as 5 << 28, not 7 << 28.

• Issue: [Free Threading] test_builtin.ImmortalTests fail on i686 (32-bit) #128058

[ZeroIntensity]

LGTM, but I'm curious about why none of our buildbots caught this. Are none of them 32 bit?

[vstinner]

Ah, the WASI job disagrees with me:

FAIL: test_immortals (test.test_builtin.ImmortalTests.test_immortals) [None]
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Lib/test/test_builtin.py", line 2702, in assert_immortal
self.assertEqual(sys.getrefcount(immortal), self.IMMORTAL_REFCOUNT)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 1879048192 != 1342177280

[vstinner]

Sorry, I don't know the difference between _Py_IMMORTAL_INITIAL_REFCNT and _Py_STATIC_IMMORTAL_INITIAL_REFCNT:

Include/refcount.h:#define _Py_IMMORTAL_INITIAL_REFCNT ((Py_ssize_t)(5L << 28))
Include/refcount.h:#define _Py_STATIC_IMMORTAL_INITIAL_REFCNT ((Py_ssize_t)(7L << 28))

I'm testing with Free Threading.

I guess I don't fully understand the issue.

[ZeroIntensity]

So it's not a typo. It's probably what I said in the issue then:

I think this is probably because there's a different reference count for heap immortals and static immortals now

[vstinner]

On 32-bit systems, it seems like a regular Python build uses 7 << 28 refcount for immortal objects, whereas the Free Threading build uses 5 << 28. I updated my PR for that.

[vstinner]

LGTM, but I'm curious about why none of our buildbots caught this. Are none of them 32 bit?

We do have 32-bit CIs but none with Free Threading.

[ZeroIntensity]

We should probably test with buildbots before merging.

[colesbury]

This is getting very confusing. In Py_REFCNT() (and sys.getrefcount()) we tried to make immortal objects in the free threading build appear to have the same refcount as in the default build:

cpython/Include/refcount.h

Lines 104 to 106 in 8a433b6

	if (local==_Py_IMMORTAL_REFCNT_LOCAL) {
	return_Py_IMMORTAL_INITIAL_REFCNT;
	}

I'm not so sure this is a good strategy anymore now that there are so many different immortal refcounts in the default build.

[ZeroIntensity]

I'm not convinced we need a different reference count for static immortals, that's going to especially complicate things.

[vstinner]

Tests / macOS / build and test (macos-13) (pull_request)Failing after 34s

That's the issue gh-128069.

[vstinner]

@colesbury:

I'm not so sure this is a good strategy anymore now that there are so many different immortal refcounts in the default build.

I'm not sure that I understand your comment correctly. Do you mean that my change is not correct? Or do you mean that you want to modify Py_REFCNT()?

[vstinner]

That's the issue #128069.

I updated the branch to retrieve the macOS fix.

[colesbury]

Do you mean that my change is not correct? Or do you mean that you want to modify Py_REFCNT()?

This fix looks good to me.

I'm not really suggesting a specific change to Py_REFCNT. Just sort of:

1. Expressing some discomfort about how the behavior of Py_REFCNT() seems complicated to me.
2. Remarking that the original motivations for the Py_REFCNT() behavior in the free threading build no longer seem relevant.

[ZeroIntensity]

Doing my best to understand the issue here. Why do we have a different immortal reference count for free threading?

[vstinner]

Merged, thanks for reviews.