This patch modernizes many modules in the stdlib by making them using
the with statement. They affected modules are modulefinder, ftplib,
cookielib, shutil, pydoc, platform, _LWPCookieJar, mailbox,
_MozillaCookieJar, and zipfile.

modulefinder should be kept compatible with Python 2.2, so please do not
apply the patch for this module.  See also PEP 291.

No idea about the other modules.

A while ago Raymond explained on Python developer list that the with
statement is slightly slower than a try/finally block. Performance
critical sections like the threading module must not use the with statement.

The patch doesn't change the threading module, so I'm not sure if
there's anything in particular you think is performance critical there.
 The places where it uses try/finally are:

  * _Condition.wait.  This performs operations on a mutex which are much
slower than any overhead "with" might bring.  It also has sleep calls in
it to implement timeout support, so it's not going to be extremely fast
in those cases anyway.  I notice, however, that it executes a number of
lines of code outside before setting up the try/finally statement,
allowing for the possibility of a buggy deadlock should an exception
(such as KeyboardInterrupt) occur before the try block begins.
  * _Event.set, which uses _Condition and so also deals with mutexes.
  * _Event.clear and _Event.wait, similarly.
  * Thread.__bootstrap, which writes to stderr and uses mutexes, so is
already quite slow. (but this case probably isn't a candidate for
conversion to with anyway)
  * Thread.__delete and Thread.join,  - more mutexes and conditions.

So there doesn't seem to be anything particularly performance critical
here.  You can argue that the slightly increased overhead of with would
still be bad, even for things that are already slow (such as mutex
manipulation), I suppose.  In this case, measuring the difference would
be a useful step to take.

However, there's also the issue of correctness.  A KeyboardInterrupt can
arrive between any of these mutxes being acquired and the try/finally
being set up.  It can also arrive after the finally begins executing and
before the mutex is actually released.  In either case, this will result
in a lock being held when it shouldn't be, which will probably lead to a
deadlock.  Converting to with to manipulate these mutexes (assuming the
lock type has its context management methods implemented in C) will
remove the possibility of these bugs occurring.

Is there a guarantee that the with-statement is safe in the face of
KeyboardInterrupt?  PEP 343 seems to imply it's not, using it as a
reason for why we need no special handling if __exit__ fails.

It's explicitly not (and Guido said this is what he wanted, several
times).  However, CPython will not raise a KeyboardInterrupt in the
middle of a C function.  Hence my parenthetical about implementing the
lock type in C.

This new patch removes the modulefinder changes.

I was the one you submitted #1910 which probably sparked the performance
debate. In this patch, I tried to avoid these touchy places.

Yes, but there's no guarantee it will even reach the C function.

There may not be a guarantee, but it will with the current
implementation, and the discussion on this ticket seems to be very
geared towards CPython implementation peculiarities.

Please make darned sure these changes are semantically neutral.

Sorry, but what does that mean? All tests are passing with the changes.

Benjamin Peterson wrote:
> Sorry, but what does that mean? All tests are passing with the changes.

Tests aren't a proof for correctness. A failing test proofs either an
error in the implementation or in the test suite. The reverse "all tests
are passing => everything is fine" is not a valid assumption.

Every change must be carefully studied. with open(name) as fp is fine in
almost every case but other cases may not.

It means to that the test suites are likely inadequate in this 
department and that you need to carefully check each transformation to 
make sure it doesn't do something subtlely different.

Today, I carefully looked through every change in my patching. I asked
myself "Does this do the same things in the same order as the original?"
and "Could exceptions cause the code to function differently?" They only
changes were when the block which used the file was not in a try finally
block so if an exception were raised, it the file would be closed. The
with statement of course removes this problem. So, I do believe the
patch is "semantically neutral." I would, however, not mind if another
set of eyes examined it.

Comments?

Lib/platform.py contains this notice at the top:

#    This module is maintained by Marc-Andre Lemburg <mal@egenix.com>.
#    If you find problems, please submit bug reports/patches via the
#    Python SourceForge Project Page and assign them to "lemburg".
#
#    Note: Please keep this module compatible to Python 1.5.2.

I didn't look into the other modules in the patch.

Please remove the platform.py part of the patch. You can apply that to
Py3k, but not to the 2.x series.

This new patch removes changes to the platform module.

Am unassigning because I don't have time for a detailed review.

For the most part, I'm not excited about this patch which targets 
modules that aren't being actively supervised by their original 
contributor.

I could break this patch into per-module patches. Then as authors
approved them, they could gradually be committed.