harvimt/quamash — pull request #13 by horazont (2014-12-17 08:45:21+00:00)
Title: Fix removal of r/w-notifiers if another notifier is still bound

I think this is a regression caused by c412687. That commit adds, a wrapper for the callback provided by the user. The workaround supplied by 43256c0 is not sufficient. The wrapper looks like this on current HEAD:

```python
def __on_notifier_ready(self, notifier, fd, callback, args):
if fd not in self._read_notifiers and fd not in self._write_notifiers:
self._logger.warning(
'Socket notifier for fd {} is ready, even though it should be disabled, not calling {} and disabling'
.format(fd, callback)
)
notifier.setEnabled(False)
return

# It can be necessary to disable QSocketNotifier when e.g. checking
# ZeroMQ sockets for events
assert notifier.isEnabled()
self._logger.debug('Socket notifier for fd {} is ready'.format(fd))
notifier.setEnabled(False)
try:
callback(*args)
finally:
notifier.setEnabled(True)
```

Assume that we have a reader for a ``fd``. Now we find out (this happens with SSL, and is handled like this e.g. in the asyncio SSL implementation) that we have to write before we can actually read, so we remove the reader from ``fd`` and add a writer for ``fd``.

The notifier for the reader gets re-enabled in the finally clause. It may re-trigger immediately, depending on what SSL did under the hoods. The test in line 405 (line 2 in the above paste) will pass, as ``fd`` is in ``_write_notifiers``.

I have provided a patch which is supposed to fix this. Take this as an idea, this is more or less a quick fix, it might be possible to do this better.