This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: [Subinterpreters]: global variable next_version_tag cause method cache bug
Type: crash Stage: resolved
Components: Subinterpreters Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: JunyiXie, vstinner
Priority: normal Keywords: patch

Created on 2021-03-09 03:54 by JunyiXie, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24822 merged JunyiXie, 2021-03-11 10:17
Messages (6)
msg388327 - (view) Author: junyixie (JunyiXie) * Date: 2021-03-09 03:54
type->tp_version_tag = next_version_tag++;
when sub interpreters parallel, next_version_tag++ is thread-unsafe. may cause different type has same tp_version_tag.

cause method cache bug in _PyType_Lookup
#define MCACHE_HASH_METHOD(type, name)                                  \
        MCACHE_HASH((type)->tp_version_tag,                     \
                    ((PyASCIIObject *)(name))->hash)

    if (MCACHE_CACHEABLE_NAME(name) &&
        _PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) {
        /* fast path */
        unsigned int h = MCACHE_HASH_METHOD(type, name);
        struct type_cache *cache = get_type_cache();
        struct type_cache_entry *entry = &cache->hashtable[h];
        if (entry->version == type->tp_version_tag && entry->name == name) {
#if MCACHE_STATS
            cache->hits++;
#endif
            return entry->value;
        }
    }



static int
assign_version_tag(struct type_cache *cache, PyTypeObject *type)
{
...
    type->tp_version_tag = next_version_tag++;
...
}
msg388331 - (view) Author: junyixie (JunyiXie) * Date: 2021-03-09 06:24
when sub interpreter finalize. 
_PyType_ClearCache set next_version_tag = 0. 

Type shared between interpreters. 
another interpreter assign_version_tag "1" for a type, the type is first assign.

the dealloc interpreter had assign_version_tag  "1" for another type.

now, two different type has same version tag. it cause method cache wrong.


static unsigned int
_PyType_ClearCache(struct type_cache *cache)
{
#if MCACHE_STATS
    size_t total = cache->hits + cache->collisions + cache->misses;
    fprintf(stderr, "-- Method cache hits        = %zd (%d%%)\n",
            cache->hits, (int) (100.0 * cache->hits / total));
    fprintf(stderr, "-- Method cache true misses = %zd (%d%%)\n",
            cache->misses, (int) (100.0 * cache->misses / total));
    fprintf(stderr, "-- Method cache collisions  = %zd (%d%%)\n",
            cache->collisions, (int) (100.0 * cache->collisions / total));
    fprintf(stderr, "-- Method cache size        = %zd KiB\n",
            sizeof(cache->hashtable) / 1024);
#endif

    unsigned int cur_version_tag = next_version_tag - 1;
    next_version_tag = 0;
    type_cache_clear(cache, 0);

    return cur_version_tag;
}
msg388332 - (view) Author: junyixie (JunyiXie) * Date: 2021-03-09 06:31
fix:
only main interpreter fini, clear method cache. 

void
_PyType_Fini(PyInterpreterState *interp)
{
    if (_Py_IsMainInterpreter(interp)) {
        clear_slotdefs();
        _PyType_ClearCache(&interp->type_cache);
    }
}

when python4.0? type isolate, each interpreter dealloc should clear method cache.
msg388495 - (view) Author: junyixie (JunyiXie) * Date: 2021-03-11 10:18
This is a simple fix.
https://github.com/python/cpython/pull/24822/commits/e61ce1dd28a48534ee497aaacb4439193bedfd42
msg388496 - (view) Author: junyixie (JunyiXie) * Date: 2021-03-11 10:21
under building Python with --with-experimental-isolated-subinterpreters
msg388619 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-13 13:38
New changeset 75048c8a38005845f10f8cb1dd33a31e0052cae0 by junyixie in branch 'master':
bpo-43441: Fix _PyType_ClearCache() for subinterpreters (GH-24822)
https://github.com/python/cpython/commit/75048c8a38005845f10f8cb1dd33a31e0052cae0
History
Date User Action Args
2022-04-11 14:59:42adminsetgithub: 87607
2021-03-13 13:39:17vstinnersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-03-13 13:38:43vstinnersetnosy: + vstinner
messages: + msg388619
2021-03-11 10:21:18JunyiXiesetmessages: + msg388496
2021-03-11 10:18:11JunyiXiesetmessages: + msg388495
2021-03-11 10:17:21JunyiXiesetkeywords: + patch
stage: patch review
pull_requests: + pull_request23588
2021-03-11 10:14:11JunyiXiesetcomponents: + Subinterpreters, - Interpreter Core
title: mutilcorevm: global variable next_version_tag cause method cache bug -> [Subinterpreters]: global variable next_version_tag cause method cache bug
2021-03-09 10:12:27JunyiXiesettype: crash
2021-03-09 06:31:20JunyiXiesetmessages: + msg388332
2021-03-09 06:24:28JunyiXiesetmessages: + msg388331
2021-03-09 03:54:44JunyiXiecreate