[Patches] [ python-Patches-450702 ] zlibmodule ALLOW_THREADS update

[noreply@sourceforge.net]

Patches item #450702, was opened at 2001-08-13 22:29
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=450702&group_id=5470

Category: Modules
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 5
Submitted By: Titus Brown (titus)
Assigned to: Martin v. Löwis (loewis)
Summary: zlibmodule ALLOW_THREADS update

Initial Comment:
When using e.g. the gzip module in a threaded
Python embedding (PyWX, pywx.idyll.org) all other
Python threads halt, because no thread interleaving
is done by the time-intensive commands in zlib.so.
This leads to serious lags when compressing 5 MB files...

I have wrapped all of the inflate and deflate commands
in Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS,
which fixes this problem.  Note that this fix is
backwards compatible to 2.1, at least, as zlibmodule.c
has not changed since then.

As requested, a _context_ diff from the head of the
current CVS tree is attached ;).


----------------------------------------------------------------------

>Comment By: Martin v. Löwis (loewis)
Date: 2001-09-07 09:28

Message:
Logged In: YES 
user_id=21627

Committed as zlibmodule.c 2.41

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2001-09-04 08:01

Message:
Logged In: YES 
user_id=21627

I fail to see the issue with the string being released from
under the compression invocation: The string is definitely
referenced from the argument tuple, and the argument tuple
is guaranteed to live as long as the function is running,
even if a thread switch occurs within the function.

This is no showstopper issue, though: the code looks correct
even with the extra incref. Unless you indicate that you'll
want to study the refcount issue in more detail and
potentially revise the patch, I'm willing to commit it
as-is.

----------------------------------------------------------------------

Comment By: Titus Brown (titus)
Date: 2001-08-20 23:55

Message:
Logged In: YES 
user_id=23486

One last patch: Alex Martelli pointed out that C functions
do not automatically take on ownership of passed-in strings,
and, under circumstances where multiple threads have access
to the same namespace, thread A might be involved in a
time-consuming command and thread B (now allowed to execute)
could then delete the input string out from under it.

Attached is the complete patch, this time including the
appropriate INCREF and DECREF of the input string.


----------------------------------------------------------------------

Comment By: Titus Brown (titus)
Date: 2001-08-17 10:10

Message:
Logged In: YES 
user_id=23486

This new patch makes the zlibmodule reentrant by using a
global zlib_lock.  Also included is the addition of
BEGIN/END_ALLOW_THREADS macros around the actual calls to
compress/decompress data, which significantly improves the
thread-friendliness of this module.

Most of the code changes are simple rearrangements of the
same old code to adapt it to the requirements of the ENTER
and LEAVE_ZLIB macros, which create new blocks; thus, any
code from which a 'return' was done had to be changed to
exit only after LEAVE_ZLIB was called.

Note that because zlib itself is re-entrant, we really only
had to worry about making methods on de/compress objects
reentrant.

----------------------------------------------------------------------

Comment By: Titus Brown (titus)
Date: 2001-08-14 12:13

Message:
Logged In: YES 
user_id=23486

N.B. I've found that zlib _is_ threadsafe.  I still need to
determine if the way in which objects are passed around in
the zlibmodule.c can potentially cause problems; it seems
like it can.


----------------------------------------------------------------------

Comment By: Titus Brown (titus)
Date: 2001-08-14 09:06

Message:
Logged In: YES 
user_id=23486

I think there may be a 2nd problem: I have to look into
some of the Python calls used to transfer data around, but
it may be possible for threads with access to the 
compression objects to retrieve data in an indeterminate 
state, i.e. mid-compression.

Luckily a module-wide lock should take care of both this 
problem AND resolve threadsafety issues with zlib!

I'll look into this & fix it.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2001-08-14 07:21

Message:
Logged In: YES 
user_id=21627

The patch looks good, however there is a major problem with
the approach taken: zlib itself might not be threadsafe.
Please try to find out whether zlib indeed is thread-safe;
if it isn't, I think you need to add another lock to prevent
multiple Python threads from accessing zlib simultaneously. 

You may want to take a look at _tkinter, which has similar
code to prevent multiple Python threads from accessing Tk
simultaneously.

----------------------------------------------------------------------

You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=450702&group_id=5470