Make _warnings.c thread-safe in free-threaded build

Feature or enhancement

The warnings implementation is split between Python (warnings.py) and C (_warnings.c). There are a few bits of code in the C module that are not thread-safe without the GIL:

The Py_SETREF calls are not thread-safe if concurrent writers try to overwrite the same field (e.g., st->once_registry). We can probably use critical sections to make it thread-safe.

get_once_registry:

cpython/Python/_warnings.c

Line 259 in df4784b

Py_SETREF(st->once_registry, registry);

get_default_action:

cpython/Python/_warnings.c

Line 290 in df4784b

Py_SETREF(st->default_action, default_action);

get_filter:

cpython/Python/_warnings.c

Line 315 in df4784b

Py_SETREF(st->filters, warnings_filters);

Some uses of borrowed references are likely not thread-safe

_PyDict_GetItemWithError (replace with PyDict_GetItemRef?)

Linked PRs

I'd like to take a stab at this.

FYI: My CPython time is very fragmented these days, so I'll move forward with a handful of smaller PRs instead of one PR that fixes everything.

gh-116664: Ensure thread-safe dict access in _warnings (#116768)

For the module state setref operations: AFAICS all of these happen in warn_explicit()¹ helpers, so perhaps we can get away with running the warn_explicit() calls as critical sections.

the C function, not the Python API ↩

pythongh-116664: Make module state Py_SETREF's in _warnings thread-safe

pythongh-116664: Ensure thread-safe dict access in _warnings (python#…

gh-116664: Make module state Py_SETREF's in _warnings thread-safe (#1…

BTW, don't we need to protect _warnings._filters_mutated?

cpython/Python/_warnings.c

Lines 1169 to 1182 in 0fa571d

    
           warnings_filters_mutated_impl(PyObject *module) 
        
           /*[clinic end generated code: output=8ce517abd12b88f4 input=35ecbf08ee2491b2]*/ 
        
           { 
        
               PyInterpreterState *interp = get_current_interp(); 
        
               if (interp == NULL) { 
        
                   return NULL; 
        
               } 
        
               WarningsState *st = warnings_get_state(interp); 
        
               if (st == NULL) { 
        
                   return NULL; 
        
               } 
        
               st->filters_version++; 
        
               Py_RETURN_NONE; 
        
           }

Possibly also the already_warned(...) calls, since that function reads st->filters_version.

(Reopening; #116959 introduced compilation warnings on non-free-treaded builds.)

_warnings._filters_mutated: Yes, I think so. Although I think the whole way it's used in catch_warnings is not really thread-safe even with the GIL.

already_warned(...): I think the calling code already does the locking? It would be good to add an assertion that it's locked at least.

pythongh-116664: Fix unused var warnings in _warnings.c in non-free-t…

_warnings._filters_mutated: Yes, I think so. Although I think the whole way it's used in catch_warnings is not really thread-safe even with the GIL.

👍 BTW, I'm a little bit sceptical about using the @critical_section Argument Clinic directive here, since that uses the module object, and not the mutex in the module state, to lock the critical section. Perhaps it does not matter.

already_warned(...): I think the calling code already does the locking? It would be good to add an assertion that it's locked at least.

Yep, an assert would be good.

gh-116664: Fix unused var warnings in _warnings.c in non-free-threade…

gh-116664: In _warnings.c, make filters_version access thread-safe (#…

Squashed commit of the following:

pythongh-116664: Ensure thread-safe dict access in _warnings (python#…

pythongh-116664: Make module state Py_SETREF's in _warnings thread-sa…

pythongh-116664: Fix unused var warnings in _warnings.c in non-free-t…

pythongh-116664: In _warnings.c, make filters_version access thread-s…

Navigation Menu

Repository navigation

Make `_warnings.c` thread-safe in free-threaded build #116664

Feature or enhancement

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	warnings_filters_mutated_impl(PyObject *module)
	/[clinic end generated code: output=8ce517abd12b88f4 input=35ecbf08ee2491b2]/
	{
	PyInterpreterState *interp = get_current_interp();
	if (interp == NULL) {
	return NULL;
	}
	WarningsState *st = warnings_get_state(interp);
	if (st == NULL) {
	return NULL;
	}
	st->filters_version++;
	Py_RETURN_NONE;
	}

Make _warnings.c thread-safe in free-threaded build #116664

Description

Feature or enhancement

Linked PRs

Activity

erlend-aasland commented on Mar 12, 2024

erlend-aasland commented on Mar 13, 2024

erlend-aasland commented on Mar 18, 2024

Footnotes

erlend-aasland commented on Mar 29, 2024

erlend-aasland commented on Mar 29, 2024

colesbury commented on Mar 29, 2024

erlend-aasland commented on Mar 29, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Make `_warnings.c` thread-safe in free-threaded build #116664