classification
Title: eventlet broke by python 2.7.x
Type: Stage:
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Denis.Bilenko, alex, barry, benjamin.peterson, christian.heimes, dstufft, giampaolo.rodola, janssen, lemburg, ncoghlan, pitrou, rkuska
Priority: normal Keywords:

Created on 2014-09-18 21:21 by alex, last changed 2016-03-15 04:21 by ncoghlan. This issue is now closed.

Messages (22)
msg227067 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2014-09-18 21:21
https://github.com/eventlet/eventlet/issues/135
msg227069 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2014-09-18 21:21
It looks like something was removed from the `_ssl` module; is that considered an implementation detail, or does it need to be added back?
msg227070 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2014-09-18 21:22
(It didn't have any direct tests as far as I can tell, which is why I didn't catch the "regression")
msg227071 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-09-18 21:23
Certainly not an official API anyway. Let eventlet get their stuff straight.
msg227079 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2014-09-19 02:39
I tend to agree.  I don't even think it was documented.  I wonder though if it makes sense to at least mention this in the PEP and/or release notes for 2.7.9.
msg227127 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2014-09-19 21:37
Definitely a case of eventlet playing with fire (private apis).
msg230773 - (view) Author: Denis Bilenko (Denis.Bilenko) Date: 2014-11-07 09:30
gevent's ssl support is also broken by 2.7.9.

https://github.com/gevent/gevent/issues/477

IMO, it is totally unexpected to have some API (even though it's undocumented and internal) removed in non-major release.

Even though both gevent and eventlet can be fixed, there still be combinations of versions that break (python >= 2.7.9 & gevent <= 1.0.1)

Please put _ssl.sslwrap back. It would save a lot of people a lot of time. I don't mind fixing gevent not to use it, but there's nothing I can do about versions already released.
msg230774 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2014-11-07 09:45
Private API is expected to be allowed to be deleted or incompatibly changed in any micro release.
msg230779 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2014-11-07 10:08
Even though it may have been considered a private API (*), users certainly won't understand that their application just broke because of a Python patch level release upgrade, so if possible, I think the API should be added back and flagged as "private, but used by at least eventlet and gevent".

Alternatively: Why not add it back and make it an officially documented API ?

(*) The API is missing the leading underscore, so it's not really private by our usual conventions, it's just undocumented.
msg230780 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2014-11-07 10:11
_ssl has leading underscore.
Privateness is "inherited", so both A._B.C and A._B._D are private.
msg230786 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2014-11-07 10:30
On 07.11.2014 11:12, Arfrever Frehtes Taifersar Arahesis wrote:
> 
> Arfrever Frehtes Taifersar Arahesis added the comment:
> 
> _ssl has leading underscore.
> Privateness is "inherited", so both A._B.C and A._B._D are private.

No, the use of the underscore in _ssl is per convention that C
implementation part of stdlib modules are moved into modules that
start with an underscore. This doesn't mean that the APIs in
those modules are private, otherwise many C implementations we have
in the stdlib would be private :-)

Also note that _ssl.sslwrap is special in that it's the main
interface between _ssl and ssl.

BTW: The sslwrap_simple() API was also removed in 2.7.9.

Note: Any libraries that need to monkey patch the Python
network stdlib will need access to these APIs. Given that the
ssl implementation changed a lot in 2.7.9, I think special care
has to be taken not to break too many of these. Using gevent
and eventlet as test cases for whether backwards compatibility
is good enough sounds like a workable approach, IMO.
msg230789 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2014-11-07 10:52
> No, the use of the underscore in _ssl is per convention that C
> implementation part of stdlib modules are moved into modules that
> start with an underscore. This doesn't mean that the APIs in
> those modules are private, otherwise many C implementations we have
> in the stdlib would be private :-)

The non-private C-implemented modules are these:

$ cd /usr/lib64/python2.7/lib-dynload ; echo [^_]*.so
array.so audioop.so binascii.so bz2.so cmath.so cPickle.so crypt.so cStringIO.so datetime.so dbm.so fcntl.so future_builtins.so gdbm.so grp.so itertools.so linuxaudiodev.so math.so mmap.so nis.so operator.so ossaudiodev.so parser.so pyexpat.so readline.so resource.so select.so spwd.so strop.so syslog.so termios.so time.so unicodedata.so zlib.so

_[^_]-prefixed, undocumented modules (amongst whom are both _[^_].py and _[^_].so) should be treated as private modules for usage only by public modules in standard library.

(_winreg is the only _[^_]-prefixed, documented module in CPython 2.7.)
msg230792 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2014-11-07 11:26
On 07.11.2014 11:52, Arfrever Frehtes Taifersar Arahesis wrote:
> 
> Arfrever Frehtes Taifersar Arahesis added the comment:
> 
>> No, the use of the underscore in _ssl is per convention that C
>> implementation part of stdlib modules are moved into modules that
>> start with an underscore. This doesn't mean that the APIs in
>> those modules are private, otherwise many C implementations we have
>> in the stdlib would be private :-)
> 
> The non-private C-implemented modules are these:
> 
> $ cd /usr/lib64/python2.7/lib-dynload ; echo [^_]*.so
> array.so audioop.so binascii.so bz2.so cmath.so cPickle.so crypt.so cStringIO.so datetime.so dbm.so fcntl.so future_builtins.so gdbm.so grp.so itertools.so linuxaudiodev.so math.so mmap.so nis.so operator.so ossaudiodev.so parser.so pyexpat.so readline.so resource.so select.so spwd.so strop.so syslog.so termios.so time.so unicodedata.so zlib.so
> 
> _[^_]-prefixed, undocumented modules (amongst whom are both _[^_].py and _[^_].so) should be treated as private modules for usage only by public modules in standard library.
> 
> (_winreg is the only _[^_]-prefixed, documented module in CPython 2.7.)

I think you're misunderstanding: just because an API is implemented
in one of the _module.c modules, doesn't imply that those APIs are
private. We have for a long time now used the approach to have a Python
module as main entry point and the _module.c modules providing the
C level bits needed by the Python module.

Regardless of whether the API can be considered private or not, packages
which have to monkey patch the low-level APIs in the network modules
will need access to those APIs in order to tap into the layers and
the sslwrap() function was/is such a low level API.

Also note that due to the major implementation changes in the ssl
module for 2.7.9 such quirks are actually expected and so it's good
that we are getting reports from eventlet and gevent on these
issues. After all, what use is a safer ssl module if your application
doesn't work anymore ;-)
msg230793 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2014-11-07 11:37
Monkey-patching is as supported as using private API.
Maintainers of third-party projects monkey-patching something or using private API should expect to sporadically have to adjust their projects to new Python versions.
msg230794 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2014-11-07 11:49
On 07.11.2014 11:30, Marc-Andre Lemburg wrote:
> BTW: The sslwrap_simple() API was also removed in 2.7.9.

Scratch that. I was in the wrong work dir :-)
msg230796 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2014-11-07 11:51
On 07.11.2014 12:49, Marc-Andre Lemburg wrote:
>> BTW: The sslwrap_simple() API was also removed in 2.7.9.
> 
> Scratch that. I was in the wrong work dir :-)

Hmm, even though the API is still there, it uses _ssl.sslwrap() as
well, so it won't work anymore either.
msg230797 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2014-11-07 11:54
> Hmm, even though the API is still there, it uses _ssl.sslwrap() as
> well, so it won't work anymore either.

It was fixed in bug #22523.
msg230799 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-11-07 12:12
It's not a mere matter of putting back the code... The 3.x ssl implementation which was backported uses a slightly different approach from the 2.x implementation, so it's not obvious we can recreate an entirely compatible implementation of_ssl.sslwrap().

As a matter of fact, gevent's fix uses some frame locals hackery to lookup the caller's "self" variable, which means it probably won't work in the general case:
https://github.com/Eugeny/ajenti/commit/54442ccb2b9ee24af15500557e7dd7b2f58acb97
msg230804 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2014-11-07 12:56
On 07.11.2014 13:12, Antoine Pitrou wrote:
> 
> It's not a mere matter of putting back the code... The 3.x ssl implementation which was backported uses a slightly different approach from the 2.x implementation, so it's not obvious we can recreate an entirely compatible implementation of_ssl.sslwrap().
> 
> As a matter of fact, gevent's fix uses some frame locals hackery to lookup the caller's "self" variable, which means it probably won't work in the general case:
> https://github.com/Eugeny/ajenti/commit/54442ccb2b9ee24af15500557e7dd7b2f58acb97

Yes, that hack will probably only work for gevent.

Is there a reason why caller_self needs to be passed to
context._wrap_socket() ?

I can't even find the ssl_sock kw args used in the hack in the current
2.7.9 code. The method only has a server_name argument.
msg230805 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2014-11-07 12:59
Looking at recent comments on the gevent ticket, the 2.7.9
changes are already causing problems for people since apparently
the changes were backported to 2.7.8 by some vendors.

https://github.com/gevent/gevent/issues/477
msg230806 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2014-11-07 12:59
FWIW, that code is all significantly simplified by the patch in http://bugs.python.org/issue22559
msg261797 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-03-15 04:21
Just noting that if we decided to reconsider addressing this upstream for the benefit of folks backporting PEP 493, Robert Kuska wrote a patch to restore sslwrap for the RHEL/CentOS PEP 466 backport:

* https://git.centos.org/blob/rpms!python.git/f63228654ecef84a78c552dac832f4cd939cf584/SPECS!python.spec#L989
* https://git.centos.org/blob/rpms!python.git/f63228654ecef84a78c552dac832f4cd939cf584/SOURCES!00221-pep466-backport-sslwrap-c-ssl.patch
History
Date User Action Args
2016-03-15 04:21:16ncoghlansetnosy: + rkuska, ncoghlan
messages: + msg261797
2014-11-07 12:59:28alexsetmessages: + msg230806
2014-11-07 12:59:22lemburgsetmessages: + msg230805
2014-11-07 12:56:49lemburgsetmessages: + msg230804
2014-11-07 12:12:18pitrousetmessages: + msg230799
2014-11-07 11:54:31Arfreversetmessages: + msg230797
2014-11-07 11:51:53lemburgsetmessages: + msg230796
2014-11-07 11:49:48lemburgsetmessages: + msg230794
2014-11-07 11:37:26Arfreversetmessages: + msg230793
2014-11-07 11:26:48lemburgsetmessages: + msg230792
2014-11-07 10:52:29Arfreversetmessages: + msg230789
2014-11-07 10:30:51lemburgsetmessages: + msg230786
2014-11-07 10:11:59Arfreversetmessages: + msg230780
2014-11-07 10:08:03lemburgsetnosy: + lemburg
messages: + msg230779
2014-11-07 09:45:22Arfreversetmessages: + msg230774
2014-11-07 09:30:57Denis.Bilenkosetnosy: + Denis.Bilenko
messages: + msg230773
2014-09-19 21:37:12benjamin.petersonsetstatus: open -> closed
resolution: not a bug
messages: + msg227127
2014-09-19 05:20:36Arfreversetnosy: + Arfrever
2014-09-19 02:39:59barrysetmessages: + msg227079
2014-09-19 02:38:55barrysetnosy: + barry
2014-09-18 21:23:27pitrousetmessages: + msg227071
2014-09-18 21:22:12alexsetmessages: + msg227070
2014-09-18 21:21:53alexsetmessages: + msg227069
2014-09-18 21:21:12alexcreate