test_descr fails randomly when executed with -R :

[pablogsal]

BPO	43636
Nosy	@vstinner, @DinoV, @methane, @pablogsal
PRs	bpo-43636: Validate the version tag in _PyType_Lookup #25032

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-04-27.13:47:19.575>
created_at = <Date 2021-03-26.23:53:55.545>
labels = ['tests', '3.9', '3.10']
title = 'test_descr fails randomly when executed with -R :'
updated_at = <Date 2021-04-27.13:47:19.574>
user = 'https://github.com/pablogsal'

bugs.python.org fields:

activity = <Date 2021-04-27.13:47:19.574>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2021-04-27.13:47:19.575>
closer = 'pablogsal'
components = ['Tests']
creation = <Date 2021-03-26.23:53:55.545>
creator = 'pablogsal'
dependencies = []
files = []
hgrepos = []
issue_num = 43636
keywords = ['patch']
message_count = 13.0
messages = ['389575', '389576', '389578', '389580', '389581', '389582', '389583', '389584', '389585', '389586', '389587', '389688', '389725']
nosy_count = 4.0
nosy_names = ['vstinner', 'dino.viehland', 'methane', 'pablogsal']
pr_nums = ['25032']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue43636'
versions = ['Python 3.9', 'Python 3.10']

[pablogsal]

❯ ./python -m test test_descr -m test_slots -R 3:3
0:00:00 load avg: 0.26 Run tests sequentially
0:00:00 load avg: 0.26 [1/1] test_descr
beginning 6 repetitions
123456
test test_descr failed -- Traceback (most recent call last):
  File "/home/pablogsal/github/cpython/Lib/test/test_descr.py", line 1201, in test_slots
    c.abc = 5
AttributeError: 'C' object has no attribute 'abc'

test_descr failed

== Tests result: FAILURE ==

1 test failed:
test_descr

Total duration: 72 ms
Tests result: FAILURE

[pablogsal]

Bisecting points at:

commit ee48c7d
Author: Dino Viehland <dinoviehland@fb.com>
Date: Sat Mar 20 12:12:05 2021 -0700

bpo-43452: Micro-optimizations to PyType_Lookup (GH-24804)

The common case going through _PyType_Lookup is to have a cache hit. There are some small tweaks that can make this a little cheaper:

* The name field identity is used for a cache hit and is kept alive by the cache. So there's no need to read the hash code o the name - instead, the address can be used as the hash.

*  There's no need to check if the name is cachable on the lookup either, it probably is, and if it is, it'll be in the cache.

*  If we clear the version tag when invalidating a type then we don't actually need to check for a valid version tag bit.

[pablogsal]

Dino, could you take a look?

[DinoV]

Looking!

[pablogsal]

Seems that the problem is removing the check:

if (MCACHE_CACHEABLE_NAME(name) &&
    _PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) {

[DinoV]

It's probably worth having an assert that the version tag is valid, that'll make it easier to see what's going wrong in the cache hit case. We should have the version tag being 0 now when it's not valid. I may not be able to debug it anymore tonight, but if no will look tomorrow.

[DinoV]

And it looks like we have an entry with a 0 version, but with a valid name:

(type_cache_entry) $3 = {
version = 0
name = 0x0000000100ec44f0
value = NULL
}

[pablogsal]

I think I have it, we still neet to validate that the version tag is correct. Can you take a look at #25032 ?

[DinoV]

I think the issue here is that in assign_version_tag there's this code:

    if (type->tp_version_tag == 0) {
        // Wrap-around or just starting Python - clear the whole cache
        type_cache_clear(cache, 1);
        return 1;
    }

the return 1 is incorrect, it should be return 0 as a valid version tag hasn't been assigned.