Skip to content

Commit eb7892a

Browse files
emmatypingemmatyping
committed
Use critical sections to lock object state
This should avoid races and deadlocks.
1 parent a6c2975 commit eb7892a

File tree

4 files changed

+24
-42
lines changed

4 files changed

+24
-42
lines changed

‎Modules/_zstd/_zstdmodule.h‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,6 @@ get_zstd_state(PyTypeObject *type) {
3333
return PyModule_GetState(module);
3434
}
3535

36-
/* ------------------
37-
Global macro
38-
------------------ */
39-
#define ACQUIRE_LOCK(obj) PyMutex_Lock(&obj->lock)
40-
#define RELEASE_LOCK(obj) PyMutex_Unlock(&obj->lock)
41-
4236
extern PyType_Spec zstddict_type_spec;
4337
extern PyType_Spec zstdcompressor_type_spec;
4438
extern PyType_Spec ZstdDecompressor_type_spec;
@@ -63,9 +57,6 @@ struct _zstd_state {
6357
typedef struct {
6458
PyObject_HEAD
6559

66-
/* Thread lock for generating ZSTD_CDict/ZSTD_DDict */
67-
PyMutex lock;
68-
6960
/* Reusable compress/decompress dictionary, they are created once and
7061
can be shared by multiple threads concurrently, since its usage is
7162
read-only.
@@ -85,9 +76,6 @@ typedef struct {
8576
typedef struct {
8677
PyObject_HEAD
8778

88-
/* Thread lock for compressing */
89-
PyMutex lock;
90-
9179
/* Compression context */
9280
ZSTD_CCtx *cctx;
9381

@@ -110,9 +98,6 @@ typedef struct {
11098
typedef struct {
11199
PyObject_HEAD
112100

113-
/* Thread lock for compressing */
114-
PyMutex lock;
115-
116101
/* Decompression context */
117102
ZSTD_DCtx *dctx;
118103

‎Modules/_zstd/compressor.c‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ _get_CDict(ZstdDict *self, int compressionLevel)
135135
PyObject *capsule;
136136
ZSTD_CDict *cdict;
137137

138-
ACQUIRE_LOCK(self);
138+
Py_BEGIN_CRITICAL_SECTION(self);
139139

140140
/* int level object */
141141
level = PyLong_FromLong(compressionLevel);
@@ -189,7 +189,7 @@ _get_CDict(ZstdDict *self, int compressionLevel)
189189
cdict = NULL;
190190
success:
191191
Py_XDECREF(level);
192-
RELEASE_LOCK(self);
192+
Py_END_CRITICAL_SECTION();
193193
return cdict;
194194
}
195195

@@ -250,20 +250,26 @@ _PyZstd_load_c_dict(ZstdCompressor *self, PyObject *dict) {
250250
}
251251
/* Reference a prepared dictionary.
252252
It overrides some compression context's parameters. */
253+
Py_BEGIN_CRITICAL_SECTION(self);
253254
zstd_ret = ZSTD_CCtx_refCDict(self->cctx, c_dict);
255+
Py_END_CRITICAL_SECTION();
254256
} else if (type == DICT_TYPE_UNDIGESTED) {
255257
/* Load a dictionary.
256258
It doesn't override compression context's parameters. */
259+
Py_BEGIN_CRITICAL_SECTION2(self, zd);
257260
zstd_ret = ZSTD_CCtx_loadDictionary(
258261
self->cctx,
259262
PyBytes_AS_STRING(zd->dict_content),
260263
Py_SIZE(zd->dict_content));
264+
Py_END_CRITICAL_SECTION2();
261265
} else if (type == DICT_TYPE_PREFIX) {
262266
/* Load a prefix */
267+
Py_BEGIN_CRITICAL_SECTION2(self, zd);
263268
zstd_ret = ZSTD_CCtx_refPrefix(
264269
self->cctx,
265270
PyBytes_AS_STRING(zd->dict_content),
266271
Py_SIZE(zd->dict_content));
272+
Py_END_CRITICAL_SECTION2();
267273
} else {
268274
/* Impossible code path */
269275
PyErr_SetString(PyExc_SystemError,
@@ -327,11 +333,6 @@ ZstdCompressor_dealloc(ZstdCompressor *self)
327333
/* Py_XDECREF the dict after free the compression context */
328334
Py_XDECREF(self->dict);
329335

330-
/* Thread lock */
331-
if (PyMutex_IsLocked(&self->lock)) {
332-
PyMutex_Unlock(&self->lock);
333-
}
334-
335336
PyTypeObject *tp = Py_TYPE(self);
336337
tp->tp_free((PyObject*)self);
337338
Py_DECREF(tp);
@@ -564,7 +565,7 @@ _zstd_ZstdCompressor_compress_impl(ZstdCompressor *self, Py_buffer *data,
564565
}
565566

566567
/* Thread-safe code */
567-
ACQUIRE_LOCK(self);
568+
Py_BEGIN_CRITICAL_SECTION(self);
568569

569570
/* Compress */
570571
if (self->use_multithread && mode == ZSTD_e_continue) {
@@ -581,7 +582,7 @@ _zstd_ZstdCompressor_compress_impl(ZstdCompressor *self, Py_buffer *data,
581582
/* Resetting cctx's session never fail */
582583
ZSTD_CCtx_reset(self->cctx, ZSTD_reset_session_only);
583584
}
584-
RELEASE_LOCK(self);
585+
Py_END_CRITICAL_SECTION();
585586

586587
return ret;
587588
}
@@ -616,7 +617,7 @@ _zstd_ZstdCompressor_flush_impl(ZstdCompressor *self, int mode)
616617
}
617618

618619
/* Thread-safe code */
619-
ACQUIRE_LOCK(self);
620+
Py_BEGIN_CRITICAL_SECTION(self);
620621
ret = compress_impl(self, NULL, mode);
621622

622623
if (ret) {
@@ -627,7 +628,7 @@ _zstd_ZstdCompressor_flush_impl(ZstdCompressor *self, int mode)
627628
/* Resetting cctx's session never fail */
628629
ZSTD_CCtx_reset(self->cctx, ZSTD_reset_session_only);
629630
}
630-
RELEASE_LOCK(self);
631+
Py_END_CRITICAL_SECTION();
631632

632633
return ret;
633634
}

‎Modules/_zstd/decompressor.c‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ _get_DDict(ZstdDict *self)
3838
return self->d_dict;
3939
}
4040

41-
ACQUIRE_LOCK(self);
41+
Py_BEGIN_CRITICAL_SECTION(self);
4242
if (self->d_dict == NULL) {
4343
/* Create ZSTD_DDict instance from dictionary content */
4444
Py_BEGIN_ALLOW_THREADS
@@ -58,7 +58,7 @@ _get_DDict(ZstdDict *self)
5858

5959
/* Don't lose any exception */
6060
ret = self->d_dict;
61-
RELEASE_LOCK(self);
61+
Py_END_CRITICAL_SECTION();
6262

6363
return ret;
6464
}
@@ -175,19 +175,25 @@ _PyZstd_load_d_dict(ZstdDecompressor *self, PyObject *dict)
175175
return -1;
176176
}
177177
/* Reference a prepared dictionary */
178+
Py_BEGIN_CRITICAL_SECTION(self);
178179
zstd_ret = ZSTD_DCtx_refDDict(self->dctx, d_dict);
180+
Py_END_CRITICAL_SECTION();
179181
} else if (type == DICT_TYPE_UNDIGESTED) {
180182
/* Load a dictionary */
183+
Py_BEGIN_CRITICAL_SECTION2(self, zd);
181184
zstd_ret = ZSTD_DCtx_loadDictionary(
182185
self->dctx,
183186
PyBytes_AS_STRING(zd->dict_content),
184187
Py_SIZE(zd->dict_content));
188+
Py_END_CRITICAL_SECTION2();
185189
} else if (type == DICT_TYPE_PREFIX) {
186190
/* Load a prefix */
191+
Py_BEGIN_CRITICAL_SECTION2(self, zd);
187192
zstd_ret = ZSTD_DCtx_refPrefix(
188193
self->dctx,
189194
PyBytes_AS_STRING(zd->dict_content),
190195
Py_SIZE(zd->dict_content));
196+
Py_END_CRITICAL_SECTION2();
191197
} else {
192198
/* Impossible code path */
193199
PyErr_SetString(PyExc_SystemError,
@@ -397,7 +403,7 @@ stream_decompress(ZstdDecompressor *self, Py_buffer *data, Py_ssize_t max_length
397403
int use_input_buffer;
398404

399405
/* Thread-safe code */
400-
ACQUIRE_LOCK(self);
406+
Py_BEGIN_CRITICAL_SECTION(self);
401407

402408
if (type == TYPE_DECOMPRESSOR) {
403409
/* Check .eof flag */
@@ -584,7 +590,7 @@ stream_decompress(ZstdDecompressor *self, Py_buffer *data, Py_ssize_t max_length
584590

585591
Py_CLEAR(ret);
586592
success:
587-
RELEASE_LOCK(self);
593+
Py_END_CRITICAL_SECTION();
588594

589595
return ret;
590596
}
@@ -651,11 +657,6 @@ ZstdDecompressor_dealloc(ZstdDecompressor *self)
651657
/* Free unused data */
652658
Py_XDECREF(self->unused_data);
653659

654-
/* Thread lock */
655-
if (PyMutex_IsLocked(&self->lock)) {
656-
PyMutex_Unlock(&self->lock);
657-
}
658-
659660
PyTypeObject *tp = Py_TYPE(self);
660661
tp->tp_free((PyObject*)self);
661662
Py_DECREF(tp);
@@ -726,7 +727,7 @@ _zstd_ZstdDecompressor_unused_data_get_impl(ZstdDecompressor *self)
726727
PyObject *ret;
727728

728729
/* Thread-safe code */
729-
ACQUIRE_LOCK(self);
730+
Py_BEGIN_CRITICAL_SECTION(self);
730731

731732
if (!self->eof) {
732733
_zstd_state* const _module_state = PyType_GetModuleState(Py_TYPE(self));
@@ -748,7 +749,7 @@ _zstd_ZstdDecompressor_unused_data_get_impl(ZstdDecompressor *self)
748749
}
749750
}
750751

751-
RELEASE_LOCK(self);
752+
Py_END_CRITICAL_SECTION();
752753

753754
return ret;
754755
}

‎Modules/_zstd/zdict.c‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,6 @@ ZstdDict_dealloc(ZstdDict *self)
6565
/* Release dict_content after Free ZSTD_CDict/ZSTD_DDict instances */
6666
Py_XDECREF(self->dict_content);
6767

68-
/* Thread lock */
69-
if (PyMutex_IsLocked(&self->lock)) {
70-
PyMutex_Unlock(&self->lock);
71-
}
72-
7368
PyTypeObject *tp = Py_TYPE(self);
7469
tp->tp_free((PyObject*)self);
7570
Py_DECREF(tp);

0 commit comments

Comments
 (0)