classification
Title: Thread locks in zlib module may go wrong in rare case
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: lukasz.langa, malin
Priority: normal Keywords: patch

Created on 2020-09-07 03:14 by malin, last changed 2021-04-27 09:03 by malin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 22126 merged malin, 2020-09-07 03:17
PR 22130 merged malin, 2020-09-07 13:47
PR 22132 merged malin, 2020-09-07 13:53
Messages (3)
msg376473 - (view) Author: Ma Lin (malin) * Date: 2020-09-07 03:14
The code in zlib module:

    self->zst.next_in = data->buf;  // set next_in
    ...
    ENTER_ZLIB(self);   // acquire thread lock

`self->zst` is a `z_stream` struct defined in zlib, used to record states of a compress/decompress stream:

    typedef struct z_stream_s {
        Bytef    *next_in;  /* next input byte */
        uInt     avail_in;  /* number of bytes available at next_in */
        uLong    total_in;  /* total number of input bytes read so far */

        Bytef    *next_out; /* next output byte will go here */
        uInt     avail_out; /* remaining free space at next_out */
        uLong    total_out; /* total number of bytes output so far */
        
        ... // Other states
    } z_stream;

Setting `next_in` before acquiring the thread lock may mix up compress/decompress state in other threads.

Moreover, modify `ENTER_ZLIB` macro, don't release the GIL when the thread lock can be acquired immediately. This behavior is the same as the bz2/lzma modules.
msg392045 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-04-27 08:37
Thanks! ✨ 🍰 ✨
msg392049 - (view) Author: Ma Lin (malin) * Date: 2021-04-27 09:03
Thanks for review.
History
Date User Action Args
2021-04-27 09:03:07malinsetmessages: + msg392049
2021-04-27 08:37:33lukasz.langasetstatus: open -> closed

nosy: + lukasz.langa
messages: + msg392045

resolution: fixed
stage: patch review -> resolved
2020-09-07 13:53:26malinsetpull_requests: + pull_request21213
2020-09-07 13:47:13malinsetpull_requests: + pull_request21211
2020-09-07 03:17:21malinsetkeywords: + patch
stage: patch review
pull_requests: + pull_request21208
2020-09-07 03:14:46malincreate