cheroot icon indicating copy to clipboard operation
cheroot copied to clipboard

cheroot/makefile.py relies on missing StreamWriter._write_lock

Open flberger opened this issue 6 years ago • 8 comments

With CherryPy 18.1.0 on Python 3.6.8 on Cygwin MS Win10, I get

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/cheroot/server.py", line 1252, in communicate
    req.respond()
  File "/usr/lib/python3.6/site-packages/cheroot/server.py", line 1056, in respond
    self.server.gateway(self).respond()
  File "/usr/lib/python3.6/site-packages/cheroot/wsgi.py", line 147, in respond
    self.write(chunk)
  File "/usr/lib/python3.6/site-packages/cheroot/wsgi.py", line 226, in write
    self.req.ensure_headers_sent()
  File "/usr/lib/python3.6/site-packages/cheroot/server.py", line 1103, in ensure_headers_sent
    self.send_headers()
  File "/usr/lib/python3.6/site-packages/cheroot/server.py", line 1188, in send_headers
    self.conn.wfile.write(EMPTY.join(buf))
  File "/usr/lib/python3.6/site-packages/cheroot/makefile.py", line 34, in write
    with self._write_lock:
AttributeError: 'StreamWriter' object has no attribute '_write_lock'

for a simple test application.

Apparently cheroot relies on behaviour that has been removed from Python 3. Bug 920 in CherryPy discusses this, see https://github.com/cherrypy/cherrypy/issues/920

This is a blocker for cheroot under a current Cygwin setup, as it effectively prevents cheroot and hence CherryPy from starting up.

I gladly provide more details.

flberger avatar Mar 13 '19 18:03 flberger

@flberger

Sounds legit. Do you have any ideas/suggestions regarding fixing this? cheroot.makefile module is quite ancient and it's hard to refactor it (I've done a bit of it, though) so any help with this is highly appreciated.

webknjaz avatar Apr 08 '19 09:04 webknjaz

Also, if you know how to wire cygwin to Travis CI and/or AppVeyor, we could set that up for regression prevention.

webknjaz avatar Apr 08 '19 09:04 webknjaz

So it looks like we prefer _pyio which still has this: https://github.com/python/cpython/blob/f4efa31/Lib/_pyio.py#L1188

Perhaps, under cygwin that import fails and it falls back to io which doesn't expose it...

webknjaz avatar Apr 08 '19 09:04 webknjaz

So... Maybe we need to rely on io.BufferedWriter in this case...

webknjaz avatar Apr 08 '19 09:04 webknjaz

cc @jaraco

webknjaz avatar Apr 08 '19 09:04 webknjaz

It looks to me like it does rely on (a subclass of) io.BufferedWriter (`StreamWriter -> BufferedWriter -> io.BuferedWriter').

I don't see that property present on my BufferedWriter either on Python 3.7:

>>> import io
>>> io.BufferedWriter._write_lock
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object '_io.BufferedWriter' has no attribute '_write_lock'

(although that might be because _write_lock is assigned in the constructor)

In any case, the issue may be that io is sometimes _pyio and other times _io (the faster C implementation). Perhaps _write_lock is only available on _pyio. In any case, I can't tell what cheroot.makefile.BufferedWriter is meant to do (its docstring doesn't say), but it's clear that it has a dependency on internal implementation details of io.BufferedWriter, so that's the crux of the problem.

I suggest we work to identify why BufferedWriter.write is overridden and see if there's a way to accomplish that in a documented and tested way without relying on implementation details (or at least supporting all implementation details across all supported environments/versions).

jaraco avatar Apr 08 '19 15:04 jaraco

Yes, it's only available in _pyio

webknjaz avatar Apr 08 '19 15:04 webknjaz

I believe that it's just a legacy approach to speed-up things by manually writing chunked data into the socket. But it's quite hard to know for sure.

I'm also not sure whether preferring the pure-Python implementation is worth it in 2019.

webknjaz avatar Apr 08 '19 15:04 webknjaz