Thread locks in zlib module may go wrong in rare case

[animalize]

BPO	41735
Nosy	@gpshead, @ambv, @animalize, @miss-islington
PRs	bpo-41735: Fix thread locks in zlib module may go wrong in rare case. #22126 [3.9] bpo-41735: Fix thread locks in zlib module may go wrong in rare case #22130 [3.8] bpo-41735: Fix thread locks in zlib module may go wrong in rare case #22132 bpo-41735: Fix thread lock in zlib.Decompress.flush() may go wrong #29587 [3.9] bpo-41735: Fix thread lock in zlib.Decompress.flush() may go wrong #29588 [3.10] bpo-41735: Fix thread lock in zlib.Decompress.flush() may go wrong (GH-29587) #29811 [3.8] [3.9] bpo-41735: Fix thread lock in zlib.Decompress.flush() may go wrong (GH-29588) #29812
Files	test_flush.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = <Date 2021-11-27.00:24:26.190>
created_at = <Date 2020-09-07.03:14:46.922>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = 'Thread locks in zlib module may go wrong in rare case'
updated_at = <Date 2021-11-27.04:14:33.454>
user = 'https://github.com/animalize'

bugs.python.org fields:

activity = <Date 2021-11-27.04:14:33.454>
actor = 'malin'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2021-11-27.00:24:26.190>
closer = 'gregory.p.smith'
components = ['Library (Lib)']
creation = <Date 2020-09-07.03:14:46.922>
creator = 'malin'
dependencies = []
files = ['50445']
hgrepos = []
issue_num = 41735
keywords = ['patch']
message_count = 9.0
messages = ['376473', '392045', '392049', '406447', '407115', '407116', '407119', '407122', '407132']
nosy_count = 4.0
nosy_names = ['gregory.p.smith', 'lukasz.langa', 'malin', 'miss-islington']
pr_nums = ['22126', '22130', '22132', '29587', '29588', '29811', '29812']
priority = 'normal'
resolution = 'fixed'
stage = 'commit review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue41735'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[animalize]

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.

[ambv]

Thanks! ✨ 🍰 ✨

[animalize]

Thanks for review.

[animalize]

Sorry, I found an omission.

The previous PRs fixed the bug in these methods:

    zlib.Compress.compress()
    zlib.Decompress.decompress()

This method also has this bug, fix in PR29587 (main/3.10) and PR29588 (3.9-):

    zlib.Decompress.flush()
    
Attached file `test_flush.py` can reliably reproduce the bug.

This time I carefully checked bz2/lzma/zlib modules, it should be no problem anymore.

Gregory P. Smith should understand these codes, add him to nosy list.

[gpshead]

New changeset 7edb627 by Ma Lin in branch 'main':
bpo-41735: Fix thread lock in zlib.Decompress.flush() may go wrong (GH-29587)
7edb627

[gpshead]

New changeset 86c1265 by Ma Lin in branch '3.9':
[3.9] bpo-41735: Fix thread lock in zlib.Decompress.flush() may go wrong (GH-29588)
86c1265

[gpshead]

Thanks malin!

[miss-islington]

New changeset 57100c8 by Miss Islington (bot) in branch '3.10':
[3.10] bpo-41735: Fix thread lock in zlib.Decompress.flush() may go wrong (GH-29587) (GH-29811)
57100c8

[animalize]

Thanks for review!