[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Bruce Merry added the comment: +tzickel I'd suggest reading the discussion in issue 36051, and maybe raising a new issue about it if you still have concerns. In short, dropping the GIL in more bytes.join cases wouldn't necessarily be wrong, but it might break code that made the assumption

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

tzickel added the comment: Regarding getting the buffer and releasing the GIL, if it's wrong, why not fix other places in the code that do it, like: https://github.com/python/cpython/blob/611836a69a7a98bb106b4d315ed76a1e17266f4f/Modules/posixmodule.c#L9619 The GIL is released, the syscall

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Change by Inada Naoki : -- stage: -> resolved status: open -> closed ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

tzickel added the comment: My mistake... -- resolution: -> not a bug ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Ma Lin added the comment: I also planned to review this commit at some moment, I feel a bit unsteady about it. If an optimization needs to be fine-tuned, and may introduces some pitfalls for future code maintenance, IMHO it is best to avoid doing this kind of optimization. --

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Bruce Merry added the comment: > static_buffers is not a static variable. It is auto local variable. > So I think other thread don't hijack it. Oh yes, quite right. I should have looked closer at the code first before commenting. I think this can be closed as not-a-bug, unless +tzickel has

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Inada Naoki added the comment: > > > > perhaps add an if to check if the backing object is really mutable ? > (Py_buffer.readonly) > > Py_buffer.readonly doesn't mean immutable. You can create read only buffer from bytearray too. Current logic uses PyBytes_CheckExact. It is safe and enough

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Inada Naoki added the comment: static_buffers is not a static variable. It is auto local variable. So I think other thread don't hijack it. -- ___ Python tracker ___

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

tzickel added the comment: Also, semi related, (dunno where to discuss it), would a good .join() optimization be to add an optional length parameter, like .join(iterable, length=10), and when running in that code-path, it would skip all the calls to (PySequence_Fast which converts no list

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

tzickel added the comment: Also, in line: https://github.com/python/cpython/blob/d07d9f4c43bc85a77021bcc7d77643f8ebb605cf/Objects/stringlib/join.h#L85 perhaps add an if to check if the backing object is really mutable ? (Py_buffer.readonly) --

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Bruce Merry added the comment: Good catch! I'll take a look this week to see what makes sense for the use case for which I originally proposed this optimisation. -- ___ Python tracker

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

Change by tzickel : -- nosy: +bmerry, inada.naoki ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue39974] A race condition with GIL releasing exists in stringlib_bytes_join

New submission from tzickel : bpo 36051 added optimization to release GIL on certain conditions of bytes joining, but it has missed a critical path. If the number of items joining is less or equal to NB_STATIC_BUFFERS (10) than static_buffers will be used to hold the buffers.