www.cendio.com
Bug 4120 - Upgrade subprocess.py or start using the system version
: Upgrade subprocess.py or start using the system version
Status: CLOSED FIXED
: ThinLinc
Other
: 3.2.0
: PC Unknown
: P2 Normal
: 4.10.0
Assigned To:
:
:
: 5657 7092
:
  Show dependency treegraph
 
Reported: 2011-12-22 17:14 by
Modified: 2018-08-10 13:13 (History)
Acceptance Criteria:
* subprocess.py should no longer be included in any thinlinc package * subprocess.py (and .pyc) should be gone after an upgrade


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description From cendio 2011-12-22 17:14:51
We are shipping a fairly old version of subprocess.py. Some development has
happened since I was involved. Some things are nice (such as restarting read
upon EINTR). Others are not so nice (for example, the stupid getstatusoutput
which is available in the 3.0 version (about to be removed though, see
http://bugs.python.org/issue9922). 

In any case, we need to ship a version which is compatible with Python 2.3.
------- Comment #1 From cendio 2011-12-29 13:21:34 -------
One alternative is raising our requirements to Python 2.4 and use the system's
subprocess.
------- Comment #2 From cendio 2012-08-09 11:06:03 -------
On bug 4213, we are considering upgrading to Python 2.4. In that case, we want
to look out for possible subprocess.py bugs in 2.4.0. It was released
2004-11-30. This is what I've found:

* check_call was added in 2.5
* check_output was added in 2.7
* Popen.send_signal(), Popen.terminate(), Popen.kill() was added in 2.6

The version we are currently shipping was added 2007-07-12, from Python
release25-maint branch. Thus, we do not need to consider features added in
later versions; we are not using them. Python 2.5.0 was released 2006-10-11. 

I've checked our tree, and the only place where check_call is used is:

/home/astrand/ctc/vsm/test-tlsession

ie not in the runtime product. As when it comes to important bug fixes between
2004-11-30 and 2007-07-12, I've found:

------------------------------------------------------------------------
r38169 | astrand | 2005-01-01 10:38:57 +0100 (lör, 01 jan 2005) | 3 lines

On UNIX, when the execution of the child fails, we must waitpid() to
prevent leaving zombies.

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

------------------------------------------------------------------------
r38559 | astrand | 2005-03-03 22:10:23 +0100 (tor, 03 mar 2005) | 2 lines

Corrected bug in list2cmdline wrt backslashes. Fixes #1083306.

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

------------------------------------------------------------------------
r45234 | martin.v.loewis | 2006-04-10 17:55:37 +0200 (mån, 10 apr 2006) | 5
lines

Patch #1467770: Add Popen objects to _active only in __del__.
Introduce _child_active member to keep track on whether a child
needs to be waited for.
Backport candidate.

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

------------------------------------------------------------------------
r46651 | georg.brandl | 2006-06-05 00:15:37 +0200 (mån, 05 jun 2006) | 2 lines

Bug #1500293: fix memory leaks in _subprocess module.

------------------------------------------------------------------------
r47077 | peter.astrand | 2006-06-22 22:21:26 +0200 (tor, 22 jun 2006) | 1 line

Applied patch #1506758: Prevent MemoryErrors with large MAXFD.
------------------------------------------------------------------------

------------------------------------------------------------------------
r50638 | peter.astrand | 2006-07-14 16:04:45 +0200 (fre, 14 jul 2006) | 1 line

Bug #1223937: CalledProcessError.errno -> CalledProcessError.returncode.
------------------------------------------------------------------------

------------------------------------------------------------------------
r50720 | georg.brandl | 2006-07-20 18:28:39 +0200 (tor, 20 jul 2006) | 3 lines

Guard for _active being None in __del__ method.


------------------------------------------------------------------------
r51758 | gustavo.niemeyer | 2006-09-06 03:58:52 +0200 (ons, 06 sep 2006) | 3
lines

Fixing #1531862: Do not close standard file descriptors in the
subprocess module.

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

Plus more. IOW, I don't think we want to use subprocess.py from 2.4.0 with TL. 


One other idea is to require, say, 2.4.3, released 2006-04-25. Critical bug
fixes between that and what we are using now are:


------------------------------------------------------------------------
r50638 | peter.astrand | 2006-07-14 16:04:45 +0200 (fre, 14 jul 2006) | 1 line

Bug #1223937: CalledProcessError.errno -> CalledProcessError.returncode.
------------------------------------------------------------------------

------------------------------------------------------------------------
r50720 | georg.brandl | 2006-07-20 18:28:39 +0200 (tor, 20 jul 2006) | 3 lines

Guard for _active being None in __del__ method.


------------------------------------------------------------------------
------------------------------------------------------------------------
r51758 | gustavo.niemeyer | 2006-09-06 03:58:52 +0200 (ons, 06 sep 2006) | 3
lines

Fixing #1531862: Do not close standard file descriptors in the
subprocess module.

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

------------------------------------------------------------------------
r53295 | peter.astrand | 2007-01-07 15:34:16 +0100 (sön, 07 jan 2007) | 1 line

Avoid O(N**2) bottleneck in _communicate_(). Fixes #1598181.
------------------------------------------------------------------------

------------------------------------------------------------------------
r53624 | peter.astrand | 2007-02-02 20:06:36 +0100 (fre, 02 feb 2007) | 1 line

We had several if statements checking the value of a fd. This is unsafe, since
valid fds might be zero. We should check for not None instead.
------------------------------------------------------------------------
------------------------------------------------------------------------
r54918 | georg.brandl | 2007-04-21 22:35:38 +0200 (lör, 21 apr 2007) | 3 lines

Bug #1704790: bind name "sys" locally in __del__ method so that it is
not cleared before __del__ is run.

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

In other wors, it seems we need to continue shipping our own subprocess.py a
little bit longer.
------- Comment #3 From cendio 2016-08-01 17:18:08 -------
Bug 4213 has been solved and we're now requiring Python 2.4, but we're still
shipping our own subprocess module. What are the remaining obstacles for
starting to use the system subprocess?
------- Comment #5 From cendio 2018-06-15 16:33:36 -------
We could move the imports in a lot of files to before we modify sys.path now,
but I'm not sure it's worth the noise.
------- Comment #7 From cendio 2018-06-18 11:22:32 -------
Upgrade works fine and can't see any issues with callers of subprocess. Should
be good to go.
------- Comment #8 From cendio 2018-08-09 16:06:22 -------
> * subprocess.py should no longer be included in any thinlinc package

Yup.

> # dpkg -L $(dpkg -l | grep thinlinc | awk '{print $2}') | grep subprocess
> # 

> * subprocess.py (and .pyc) should be gone after an upgrade

Yup.

Tested on a 32-bit Debian 9 VM with tl-4.9.0->4.9.0post-5867.
------- Comment #9 From cendio 2018-08-10 13:13:39 -------
As a note, it's unclear why a different approach to removing the .pyc file was
taken compared to the removal of xmlrpclib, where tl-setup removes the .pyc
file.

I would have preferred a single approach, but I'm not going to be picky about
it and reopen the bug. It's fine for now.