Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.1k
Closed
Description
Bug report
EDIT: edited to clarify that the issue is in the C implementation ofoperator.methodcaller.
Originally reported by@ngoldbaum incrate-py/rpds#101
Reproducer
fromoperatorimportmethodcallerfromconcurrent.futuresimportThreadPoolExecutorclassHashTrieMap():defkeys(self):returnNonedefvalues(self):returnNonedefitems(self):returnNonenum_workers=1000views= [methodcaller(p)forpin ["keys","values","items"]]defwork(view):m,d=HashTrieMap(), {}view(m)view(d)iterations=10for_inrange(iterations):executor=ThreadPoolExecutor(max_workers=num_workers)forviewinviews:futures= [executor.submit(work,view)for_inrange(num_workers)]results= [future.result()forfutureinfutures]
Once every 5-10 runs, the program prints:
TypeError: descriptor 'keys' for 'dict' objects doesn't apply to a 'HashTrieMap' objectThe problem is thatoperator.methodcaller is not thread-safe because it modifies thevectorcall_args, which is shared across calls:
Lines 1646 to 1666 in0af4ec3
| staticPyObject* | |
| methodcaller_vectorcall( | |
| methodcallerobject*mc,PyObject*const*args,size_tnargsf,PyObject*kwnames) | |
| { | |
| if (!_PyArg_CheckPositional("methodcaller",PyVectorcall_NARGS(nargsf),1,1) | |
| || !_PyArg_NoKwnames("methodcaller",kwnames)) { | |
| returnNULL; | |
| } | |
| if (mc->vectorcall_args==NULL) { | |
| if (_methodcaller_initialize_vectorcall(mc)<0) { | |
| returnNULL; | |
| } | |
| } | |
| assert(mc->vectorcall_args!=0); | |
| mc->vectorcall_args[0]=args[0]; | |
| returnPyObject_VectorcallMethod( | |
| mc->name,mc->vectorcall_args, | |
| (PyTuple_GET_SIZE(mc->xargs)) |PY_VECTORCALL_ARGUMENTS_OFFSET, | |
| mc->vectorcall_kwnames); | |
| } |
I think this is generally unsafe, not just for free threading. Thevectorcall args array needs to be valid for the duration of the call, and it's possible formethodcaller to be called reentrantly or by another thread while the call is still ongoing.