python / asyncio Goto Github PK

can_write_eof should return simple constant: True for plain socket and False 
for ssl

Unlike most other objects defined in tulip, Queue classes aren't importable at 
the toplevel.

This wasn't discussed in the code review. I'm filing this ticket in case it's 
an oversight.

Trivial patch attached.

What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?


Please use labels and text to provide additional information.

Andrew suggests adding some info to logs:

proactor_events.py:183: tulip_log.exception('Accept failed')
Add info about connection address

selector_events.py:112: tulip_log.exception('Accept failed')
Add extra info: self address and peer address

I imagine there are other log messages that could be extended.

Sometimes you write foo() instead of yield from foo().  If you use the result, 
you probably get a type error pretty quickly.  But if you don't use the result, 
you have a pretty nasty quiet failure on your hand.

Maybe we can solve this by having a __del__ method in a wrapper object that 
checks whether the generator has been consumed at all, and a similar __del__ 
method on Future.  It would have to log an error.

Similarly, it would also be useful if a Future that completed with an exception 
logged an error if it was garbage-collected before anybody looked at the error. 
 (I tried to do this once but I couldn't get it to work right; see attached 
EXCEPT.diff.)

[deleted issue]

Tulip has Lock, EventWaiter, Condition, and Semaphore. I propose it should have 
Queue, JoinableQueue, LifoQueue, and PriorityQueue. I offer to implement them. 
I'm starting here:

http://code.google.com/r/jesse-tulip/source/browse/tulip/queues.py

http://code.google.com/r/jesse-tulip/source/browse/tests/queues_test.py

I'll submit a patch in a couple days or weeks when they're solid.

We need more efficient version of IocpProactor.stop_serving()
Right now it is O(N).

close should not cancel _write_fut, _loop_writing should schedule 
_call_connection_lost if _closing is set to True.

Even as current development is not worried about install, the present  etup.py 
file, necessary to build the Windows only '_overlapped' extension will break if 
someone download the code and tries "python setup.py build" (or install) - 
instead of playing with the tree inplace.

A suggestion is to upgrad de setup.py to a minimal one that won't break when 
setup install is called.

The attached patch is a suggestion for the needed changes. Please note I don't 
have access to a windows box right now to know for certain if it is properly 
building the C extension as is. (I tried it with a stub extension instead and 
it worked.)

The event loop part and the coroutines are clearly separated in the 
specification. I understand that the latter requires Python 3 (because of yield 
from syntax) however I assume that the event loop specification could be 
implemented with Python 2, too.

What's the reason for explicitly excluding Python 2 support?

Everything else should start with an _underscore.

(Quoting Stephen Thorne:)

I don't think it's worth having a different API functions to distinguish 
between client and server datagram protocols.

For instance, this function is will do precisely what is needed and no more:
  def start_serving_datagram(protocol_factory, local_addr=None, remote_addr=None, *, family=0, proto=0, flags=0)
    s = socket.socket(family, proto, flags)
    if local_addr:
      s.bind(local_addr) # FIXME: name resolution
    if remote_addr:
      s.connect(remote_addr) # FIXME: name resolution
    # It is, in fact, not necessary to even call bind() *or* connect(), a port will be automatically bound on the first sendto().
    ...

The current implementation of _SelectorDatagramTransport.sendto(data, 
addr=None) should work fine.

(Guido speaking:)
I'm not sure what the name of this method should be.  Maybe 
create_datagram_thingie()? :-)

Note that this assumes local_addr and remote_addr are (host, port) tuples.  The 
alternative would be to have 4 arguments with somewhat complex rules for which 
could be None.  Also note that we should call yield from self.getaddrinfo(...) 
on each address to avoid blocking for the name lookup in the bind() or 
connect() call.  Finally we need to research whether the connect() (for a UDP 
socket) could ever block on any OS -- if so, we'd need to use yield from 
self.sock_connect(...) instead.

Try this little program.  It runs till completion and no traceback is printed 
for the ZeroDivisionError that is happening.

from tulip import *

@task
def go():
    print('hiss')
    yield from sleep(1)
    print('boom')
    1/0


def main():
    l = get_event_loop()
    go()
    l.run_until_complete(sleep(2))

main()

Every test that has a sleep or timeout is a potential liability when run on a 
slow or busy system.  We could increase the time to sleep (10 msec is very 
short) but that would make the tests run super slow (they currently take 10 
seconds for ~800 tests, and 60% of that seems to be sleeping).  Maybe we can 
run tests in parallel or have a virtual clock?

errno checking is more verbose and less robust.

I keep getting InvalidStateError when using run_until_complete() with a 
timeout, but can't understand why.  The minimal example I can come up with is 
the following:

import tulip
loop = tulip.get_event_loop()

for i in range(2):
    f = tulip.async(tulip.sleep(1))
    try:
        # next line raises InvalidStateError on second loop
        loop.run_until_complete(f, timeout=0.1)
    except tulip.TimeoutError:
        # commenting out next line cures problem
        f.cancel()
    else:
        raise RuntimeError('expected timeout')

We have to decide what to do with bytearray in
Transport.write() and sendto() methods.

Should we allow bytearray?

When WSGIServerHttpProtocol is instantiated with readpayload=True, 
environ['wsgi.input'].read() will always return an empty bytestring instead of 
the request payload.

I'm attaching a fix with a test.

There's been some good talk at PyCon about the danger of using subclassing as 
an API style, and recommending composition of objects instead.  We should 
review all our APIs to see if we're in danger of falling in this trap.  It is 
one of the reasons asyncore is such a disaster.

See e.g. 
https://code.google.com/p/billiejoex/source/browse/works/tulip_ftpd.py?spec=svn4
a9c273418c2f746a65f789d1eba7681cc09bdd6&r=4a9c273418c2f746a65f789d1eba7681cc09bd
d6

call_in_thread() gets back a future from ThreadRunner.submit() to which it adds 
the done callback task.unblock_if_alive().  This will be executed in the thread 
pool *after* the other done callback which wakes up the pollster.

If the pollster awakes before task.unblock_if_alive() is called then the 
eventloop can exit because there appears to be no work left.

If the order of invocation of the callbacks were reversed then I think that 
would fix the race.

This comment in base_events.py:

        # Inspect the poll queue.  If there's exactly one selectable
        # file descriptor, it's the self-pipe, and if there's nothing
        # scheduled, we should ignore it.

makes me nervous.  The self-pipe exists to avoid race conditions between the 
event loop and a thread running e.g. getaddrinfo().  I don't see why we should 
avoid polling the self-pipe if it's the only one -- we may open up the race 
condition.

I suspect this code exists because of the desired semantics for run(), which is 
to stop when the self-pipe is the only thing left.  But I think we need to do 
it more carefully.

Now _UnixWritePipeTransport registers itself in poller only if it has pending 
writes.

So when there are no such writes transport did not catch disconnects.

Now eventloop has no way to register ERROR event for file descriptor.

Should we add EVENT_ERROR type and add_error/remove_error functions to 
AbstractEventLoop?

BTW EpollSelector should to process EPOLLHUP as well (always adding this flag 
by default makes sense).

This issue is blocker for subprocess support in tulip.

right now we expose public api at namespace package level.
for example "tulip.Task" or "tulip.http.WSGIServerHttpProtocol"

Disclaimer: I don't know if you're interested in feedback on Tulip at this 
point. If you aren't, just tell me, I'll stop bugging you until you release an 
alpha :)

Context: I'm testing a WebSocket client running on Tulip against the AutoBahn 
test suite (which runs on Twisted) on OS X.

Problem: I encounter the following error:

Fatal error for <tulip.selector_events._SelectorSocketTransport object at 
0x10c0c1b50>
Traceback (most recent call last):
  File "/Users/myk/Documents/dev/tulip/tulip/selector_events.py", line 336, in _read_ready
    data = self._sock.recv(16*1024)
ConnectionResetError: [Errno 54] Connection reset by peer

Analysis:

`_SelectorSocketTransport._read_ready` expects `recv` to return an empty 
bytestring when the end of the stream is reached. In this case, `recv` raises 
an exception instead.

I could confirm that the connection is being closed in `KqueueSelector.select`, 
because at that point `kev.flags` has the `KQ_EV_EOF` bit set. But that doesn't 
appear to be the right place to handle this condition.

I think this problem has to do with the way the TCP connection is shut down. I 
haven't managed to reproduce the problem with other servers. Still, it 
shouldn't result in a fatal exception.

Reproduction:

Here's the simplest way to trigger the exception I have to offer — sorry :/

In a shell:

% mkvirtualenv --python=python2.7 autobahn
% pip install autobahntestsuite
% wstest -m fuzzingserver

In another shell:

% python3.3 dummy_client.py        # see below

After a few seconds wstest times out, closes the connection and dummy_client.py 
displays the stack trace.

------------------------------------------------------------------------
# dummy_client.py

import tulip

class DummyProtocol(tulip.Protocol):
    def connection_made(self, transport):
        self.transport = transport
        self.stream = tulip.StreamReader()

    def data_received(self, data):
        self.stream.feed_data(data)

    def eof_received(self):
        self.stream.feed_eof()
        self.transport.close()

loop = tulip.get_event_loop()

def main():
    trans, proto = yield from loop.create_connection(
            DummyProtocol, 'localhost', 9001)
    yield from proto.stream.read()

loop.run_until_complete(main())
------------------------------------------------------------------------

Proactor methods which return futures could return a subclass of Future whose 
cancel() method also invokes the overlapped objects cancel() method.

Currently the overlapped operation is allowed to continue but the result is 
ignored.

As reported by Guido (and I see it too) we sometimes get the following failure

> Hm, I still occasionally see that KeyError, and I also saw this once:
> 
> Skipping 'subprocess_test': No module named 'fcntl'
> Skipping 'unix_events_test': No module named 'fcntl'
> .............................................s.....Exception in callback
> <bound method _ProactorSocketTransport._loop_re
> ading of <tulip.proactor_events._ProactorSocketTransport object at
> 0x032A68D0>> (Future<result=b''>,)
> Traceback (most recent call last):
>    File "C:\Users\guido\tulip\tulip\events.py", line 39, in _run
>      self._callback(*self._args)
>    File "C:\Users\guido\tulip\tulip\proactor_events.py", line 65, in
> _loop_reading
>      self._protocol.eof_received()
>    File "tests\events_test.py", line 51, in eof_received
>      assert self.state == 'CONNECTED', self.state
> AssertionError: CLOSED

I think this is because the check for self._closing in _loop_reading() is 
bypassed if EOF is encountered.

See this thread:
https://groups.google.com/forum/#!topic/python-tulip/hr1kPZfMX8U

Even if we don't remove get_event_loop() and set_event_loop(), the tulip code 
itself should not need to call it whenever an event loop is already available.  
For example, any code calling Future() from inside some code that has a loop 
parameter or instance variable should be modified to pass that loop on to the 
Future constructor instead of letting the latter call get_event_loop().

Also, calling set_event_loop(None) explicitly should not cause a subsequent 
call to get_event_loop() to create a new event loop instance.

I have work in progress for this:
https://codereview.appspot.com/12637044/

What steps will reproduce the problem?
1. Run one of the following examples mpsrv.py srv.py wssrv.py
2. Make a request
3.

What is the expected output? What do you see instead?

The request succeeds, but at the end of the request self.close() is called:
ERROR:root:Error handling request
Traceback (most recent call last):
  File "/home/andi/opt/tulip/tulip/http/server.py", line 127, in start
    yield from handler
  File "/home/andi/opt/tulip/tulip/tasks.py", line 29, in coro
    res = func(*args, **kw)
  File "mpsrv.py", line 103, in handle_request
    self.close()
AttributeError: 'HttpServer' object has no attribute 'close'

The close() method was removed in revision 9cb4f18b3719.

I've uploaded a proposal to fix the examples here: 
https://codereview.appspot.com/9528043/

The change is also available in my clone of the tulip repository: 
https://code.google.com/r/albrechtandi-tulip/source/detail?r=614566965638add0550
a68f9ce8adadd985dff1d


What version of the product are you using? On what operating system?


Please provide any additional information below.

When @coroutine is used to wrap something that's not a generator, it should 
assume that the user meant to write a generator but just didn't have any reason 
to yield -- maybe it's a stub, maybe the yield is temporarily commented out, 
etc.

In this case @coroutine should make up a wrapper that *is* a coroutine and 
which just calls the wrapped function.

There's one odd edge case: it's possible that the wrapped function itself is 
not a generator, but calling it returns a generator.  The wrapper should 
dynamically deal with this -- it can just "yield from result" if the result is 
a generator.

Note: in the future we could introduce a debug mode where @coroutine always 
does something.

Now we push everything into global logging namespace, should to use 'tulip' 
logger for that purposes. 

When some code that is itself a callback (e.g. a fd handler) wants to call some 
other code (maybe a protocol method) immediately before returning, I used to 
use call_soon() to delay the other call.  I think this just causes more work 
going through the event loop.  I think it's safe to make such calls directly, 
provided that

1. It is the last call before returning

2. There is no try/execpt or try/finally surrounding it

On OSX the default number of file descriptors is only 256. Would be nice if an 
app that opens lots of file descriptors would automatically be slowed down 
instead of getting exceptions if you run out of theses...

This should act both as proof that it can be done, as an example of how it 
should be done, and as a practical suggestion how to write responsive UI apps 
using Tkinter.  But it should probably be kept out of PEP 3156.

I’ll replicate a Mail to or benevolent dictator below.

This bug is about the fact that the tulip API isn’t pythonic, as it uses 
Java-style code: The most +1’d comment in GvR’s post about needing experts 
in async is by me: 
https://plus.google.com/u/0/115212051037621986145/posts/fnb8pqsGGb2

I begged him not to add more java code to the Python stdlib, but unfortunately 
PEP 3156 is exactly that in parts.

E.g. “ev.add_signal_handler”. Python has __get/setitem__, so why don’t we 
use it? also the “add” part is extremely misleading as it’s in reality 
“set”, since the old handler is replaced. it’s a setter → unpythonic.

why not “ev.signal_handlers[sig] = callback” (call without arguments) and 
“ev.signal_handlers[sig] = (callback, *args)”, as well as “del 
ev.signal_handlers[sig]”? that would be so much more pythonic. also we could 
easily get a dict of currently registered handlers via 
“ev.signal_handlers”, and one registered handler via 
“ev.signal_handlers[sig]”.

The same for readers and writers. It seems that the current API is the 
equivalent of “Programmer art” and not intended for general consumption.

I’d strongly hope for a pythonificating revision.

There should only be two:

- run() runs forever (until stop() is called)

- run_until_complete(f) runs until future f is complete

For now test doesn't rely on network connection, but I see suspicious delays 
when run unittests suite.

Have to look on, maybe we have extra calls for event_loop.run_once etc in our 
tests.

I am tired of writing self.event_loop where self.loop would do.  Some code 
already uses just loop.  Let's use loop for all new code, and fix existing code 
opportunistically, one file at a time.

Note: I don't think we should change the official APIs.  Just local and 
instance variables used to hold the current event loop.

What steps will reproduce the problem?

$ python runtests.py
...
Fatal error for <tulip.selector_events._SelectorSocketTransport object at 
0x7f7f2406b350>
Traceback (most recent call last):
  File "/home/andrew/projects/tulip/tulip/http/server.py", line 91, in start
    info = yield from self.stream.read_request_line()
  File "/home/andrew/projects/tulip/tulip/http/protocol.py", line 95, in read_request_line
    raise errors.BadStatusLine(line) from None
tulip.http.errors.BadStatusLine: ''

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/andrew/projects/tulip/tulip/selector_events.py", line 372, in write
    n = self._sock.send(data)
BrokenPipeError: [Errno 32] Broken pipe
...

I can reproduce it on my Ubuntu linux box sometimes.
Not every run but often enough.

Should we process BrokenPipeError as well as other disconnection errors for 
this case?

The blocking sequence of commands are:

s = socket.socket()
s.bind(local_address)
s.connect(remote_address)
...

The current api create_connection() does not provide the ability to provide a 
'local' address to bind to. Bind must be called between socket.socket() and 
sock.connect(), so it is appropriate to have it inside the create_connection() 
call or document clearly that create_connection(sock=sock) is preferred for 
anything like this.

I closed issue 41 as fixed, but there's one missing piece: abandoned tracebacks 
only get printed when the Future is garbage-collected.  If nothing else is 
going on this may take a while.  Perhaps we should have some heuristic in 
_run_once() that occasionally calls gc.collect()?

Every time _run_once() is called is probably too often -- maybe we should call 
it at most once a second?

Implementation tests are for Tulip behavior specifically.  Specification tests 
are also for validating other implementations of PEP 3156.  Right now we don't 
distinguish between these categories, but we probably should.

Some tests in base_events_test.py and events_test.py are still hitting 
xkcd.com.  Hopefully we can adapt run_test_server() to fix these.

Refactor http exceptions hierarchy.
Possibly drop http.client errors dependency.
Possible rename tulip.http to something else.

     def readexactly(self, n):
         """COUROUTINE: Read exactly n bytes, or until EOF."""
         blocks = []
         count = 0
         while n > count:
             block = yield from self.read(n - count)
+            if not block:
+                break
             blocks.append(block)
             count += len(block)
         return b''.join(blocks)

Minor word cleanup.

Wrapping a function in @tulip.task doesn't preserve its docstring.

Patch:

diff -r ede95651af9f tulip/tasks.py
--- a/tulip/tasks.py    Thu Mar 28 18:38:25 2013 -0700
+++ b/tulip/tasks.py    Mon Apr 01 00:25:17 2013 +0200
@@ -56,6 +56,7 @@

 def task(func):
     """Decorator for a coroutine to be wrapped in a Task."""
+    @functools.wraps(func)
     def task_wrapper(*args, **kwds):
         coro = func(*args, **kwds)
         return Task(coro)

kqueue() and epoll() use a file descriptor per instance.
Currently, Pollster() lacks a close() method to close the file descriptor, so 
there's a risk of FD leak.

close() should be added (and probably also support the context manager 
protocol).

There are lots of possible bikesheds about method names (and other names).  
E.g. connection_made/lost doesn't sound right for UDP or pipes, the various 
run*() methods may need reshuffling, some folks think it should be set_reader() 
instead of add_reader(), etc.

This is just a reminder issue to make sure we don't forget to think about this 
some more, without being distracted by it in the short term.