Repository navigation
Conversation
| } | ||
| status = 0; | ||
|
|
||
| cleanup:; |
There was a problem hiding this comment.
| cleanup:; | |
| cleanup: |
?
There was a problem hiding this comment.
Nope, Py_END_CRITICAL_SECTION(); is not a statement :(
Without it:
src/msgspec/_core.c:13049:5: warning: label at end of compound statement is a C23 extension [-Wc23-extensions]
13049 | Py_END_CRITICAL_SECTION();
| ^
/Users/sobolev/.local/share/uv/python/cpython-3.13.5-macos-aarch64-none/include/python3.13/cpython/critical_section.h:89:5: note: expanded from macro 'Py_END_CRITICAL_SECTION'
89 | }
| ^
1 warning generated.
There was a problem hiding this comment.
Oh. A bit confusing to read though I admit 😅
But I guess you get used to it
There was a problem hiding this comment.
That's why Rust and Zig were invented 😆
There was a problem hiding this comment.
Is your code clear and easy to read?
Then we’re coming for you!
©gotoand macros
|
Moving the size read under the lock fixes the original race, but there is still a crash when another thread modifies a nested list while the outer list is being cleared. I reproduced this on Python 3.14 with the GIL disabled, using the code below without an CPython can temporarily release the outer list's lock while waiting for the inner list's lock. For example, with The next access to Reproducer for Python 3.14 with the GIL disabledimport threading
import msgspec
inner = [1, 2, 3]
outer = [inner] * 64
stop = threading.Event()
def churn_inner():
while not stop.is_set():
inner.append(0)
inner.pop()
def shrink_outer():
while not stop.is_set():
outer.clear()
outer.extend([inner] * 64)
def encode():
for _ in range(20000):
msgspec.msgpack.encode(outer)
mutators = [threading.Thread(target=f)
for f in (churn_inner, churn_inner, shrink_outer)]
encoders = [threading.Thread(target=encode) for _ in range(4)]
for t in mutators + encoders:
t.start()
for t in encoders:
t.join()
stop.set()
for t in mutators:
t.join()There is also a slowdown on regular GIL builds: |
|
@Siyet friendly ping :) |
This PR does two things:
objlist item was modified by another thread in FT build.objwas used to get the__len__before the lock. So, the size and contents were not in sync. I fixed this, now we do it under a lockRefs #1249