Data race in readline.get_completer() reads state->completer without the critical section

Bug report

Bug description:

In the free-threaded build, the readline module guards its hook state with @critical_section on ~20 functions (including set_completer), but readline.get_completer() reads state->completer and Py_NewRefs it without taking the critical section.

cpython/Modules/readline.c

Lines 894 to 903 in a74280e

    
           static PyObject * 
        
           readline_get_completer_impl(PyObject *module) 
        
           /*[clinic end generated code: output=6e6bbd8226d14475 input=6457522e56d70d13]*/ 
        
           { 
        
               readlinestate *state = get_readline_state(module); 
        
               if (state->completer == NULL) { 
        
                   Py_RETURN_NONE; 
        
               } 
        
               return Py_NewRef(state->completer); 
        
           }

set_completer writes the same field via set_hook,

cpython/Modules/readline.c

Lines 880 to 886 in a74280e

    
           static PyObject * 
        
           readline_set_completer_impl(PyObject *module, PyObject *function) 
        
           /*[clinic end generated code: output=171a2a60f81d3204 input=97f539d8d0bfcb95]*/ 
        
           { 
        
               readlinestate *state = get_readline_state(module); 
        
               return set_hook("completer", &state->completer, function); 
        
           }

which decrefs the old completer with Py_XSETREF(state->completer, ...).

cpython/Modules/readline.c

Lines 473 to 481 in a74280e

    
           static PyObject * 
        
           set_hook(const char *funcname, PyObject **hook_var, PyObject *function) 
        
           { 
        
               if (function == Py_None) { 
        
                   Py_CLEAR(*hook_var); 
        
               } 
        
               else if (PyCallable_Check(function)) { 
        
                   Py_XSETREF(*hook_var, Py_NewRef(function)); 
        
               }

Reproducer:

import readline
from threading import Thread

def setter():
    for _ in range(20000):
        readline.set_completer(lambda text, state: None)

def getter():
    for _ in range(20000):
        readline.get_completer()

threads  = [Thread(target=setter) for _ in range(2)]
threads += [Thread(target=getter) for _ in range(6)]
for t in threads: t.start()
for t in threads: t.join()

TSAN Report:

==================
WARNING: ThreadSanitizer: data race (pid=39505)
  Read of size 8 at 0x7fffb60407f8 by thread T3:
    #0 readline_get_completer_impl /cpython/./Modules/readline.c:899:16
    #1 readline_get_completer /cpython/./Modules/clinic/readline.c.h:654:12 
    #2 cfunction_vectorcall_NOARGS /cpython/Objects/methodobject.c:508:24 
    #3 _PyObject_VectorcallTstate /cpython/./Include/internal/pycore_call.h:144:11 
    #4 PyObject_Vectorcall /cpython/Objects/call.c:327:12 
    #5 _Py_VectorCallInstrumentation_StackRefSteal /cpython/Python/ceval.c:768:11 
    #6 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:1906:35
...

  Previous write of size 8 at 0x7fffb60407f8 by thread T1:
    #0 set_hook /cpython/./Modules/readline.c:480:9 
    #1 readline_set_completer_impl /cpython/./Modules/readline.c:885:12 
    #2 readline_set_completer /cpython/./Modules/clinic/readline.c.h:632:20 
    #3 _Py_BuiltinCallFast_StackRef /cpython/Python/ceval.c:817:11 
    #4 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:2510:35
...

SUMMARY: ThreadSanitizer: data race /cpython/./Modules/readline.c:899:16 in readline_get_completer_impl
==================

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

pythongh-153291: Fix data race in readline.get_completer() and get_pr…

pythongh-153291: Move the free-threaded readline tests to test_free_t…

Merge branch 'main' into pythongh-153291-readline-get-completer-race

gh-153291: Fix data race in readline.get_completer() and get_pre_inpu…

[3.15] gh-153291: Fix data race in readline.get_completer() and get_p…

Navigation Menu

Repository navigation

Data race in `readline.get_completer()` reads `state->completer` without the critical section #153291

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	static PyObject *
	readline_get_completer_impl(PyObject *module)
	/[clinic end generated code: output=6e6bbd8226d14475 input=6457522e56d70d13]/
	{
	readlinestate *state = get_readline_state(module);
	if (state->completer == NULL) {
	Py_RETURN_NONE;
	}
	return Py_NewRef(state->completer);
	}

	static PyObject *
	readline_set_completer_impl(PyObject module, PyObject function)
	/[clinic end generated code: output=171a2a60f81d3204 input=97f539d8d0bfcb95]/
	{
	readlinestate *state = get_readline_state(module);
	return set_hook("completer", &state->completer, function);
	}

	static PyObject *
	set_hook(const char funcname, PyObject hook_var, PyObject function)
	{
	if (function == Py_None) {
	Py_CLEAR(*hook_var);
	}
	else if (PyCallable_Check(function)) {
	Py_XSETREF(*hook_var, Py_NewRef(function));
	}

Data race in readline.get_completer() reads state->completer without the critical section #153291

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Data race in `readline.get_completer()` reads `state->completer` without the critical section #153291