Windows support

[ntninja]

As can be seen from the list of known issues below, this still needs some work to be really usable, but the initial implementation is done and if you have any feedback or suggestions about it I'd be happy to discuss that with you and incorporate any necessary changes.
As you study the code you'll probably notice that this is a mostly rewrite of the library, implementing a full-fledged event loop (similar to asyncio's SelectorEventLoop and ProactorEventLoop) on top of base_events.BaseEventLoop instead of somehow trying to use SelectorEventLoop for this. This IMHO makes for more readable and stable code, because we're simply implementing the well defined interfaces from the BaseEventLoop instead of monkey-patching SelectorEventLoop to fit our needs.

Known issues:

• TCP connection closings are not received with the standard GLib or GTK event loop on Windows
• TLS does not work with Python 3.4 and below (even outside of Windows)
• UDP and Pipe support missing
• Support for the add_reader and add_writer APIs is still lacking
• Unix domain sockets and signals support is also still missing
• Subprocess support is untested currently
• Not tested with anything older than Python 3.5.2

Passes most of the aiohttp test suite on Linux (except for 2 tests that I believe to be incorrect after debugging them for a couple of hours), Windows results for the test suite are not quite on par yet, but a standard HTTPS with a large number of requests and many transferred bytes has completed without errors.

[nhoad]

Hi! Sorry for the delay. First up, thanks heaps for looking into this! It's very exciting. A few questions/thoughts:

• What monkey-patching are you referring to? The code in GLibEventLoopPolicy.__init__? If so, I agree that this is not ideal, but that's easy enough to fix up - we just need to provide implementations of those methods. The gross monkey-patching is (I think, it's from before I took over the project) used as a shortcut to avoid implementing them ourselves, which can be done completely separately to your work.
• We're overriding private methods quite a lot, which I don't consider to be well-defined. How much work do you think it would be to stop doing this, and instead override the public methods that call them? If it's too much work/pain, that's okay, it just means we'll have to write a lot more tests and get more serious about our automated testing, which is something I've wanted to do for a while now anyway.
• You say that TLS does not work with Python 3.4 and below, but later on you mention that you've not tested it with anything older than 3.5.2. Is this referring to the use of the new SSL API?
• Where has gbulb/transports.py come from? This is a massive amount of code that is completely untested. A non-trivial portion of it seems to be copy-pasted from proactor_events.py, which raises licensing concerns and may cause maintenance headaches.
• asyncio.events._set_running_loop and asyncio.events._get_running_loop calls are sometimes, but not always, guarded with existence checks beforehand. Please fix up the ones that don't have such checks.
• Please ensure there are no tabs in the code. There are many in gbulb/transports.py, for example.
• The implementation of GLibBaseEventLoop.select is pretty awesome!

In future, if you could use smaller, more clearly separated commit messages, that would be good - I think most of this code is not necessarily Windows specific, so at the moment I'm having quite a bit of difficulty finding the changes that are directly necessary for Windows support.

All of that said though, I think this is pretty awesome! I've created a new branch named windows-support, if you can change your base branch to that one instead, I can start merging in changes without fear of breaking master. Once we're satisfied with the quality, I can merge it into master.

I'll start working on getting some automated testing going for major Python versions we care about. I'm not sure what we can do with Windows for automated testing though, so if you have any suggestions, I'm interested in hearing them :)

[gbtami]

Seems most project on GitHub uses appveyor for Windows automated testing. You can get the idea from https://github.com/harvimt/quamash

[ntninja]

Hi! Sorry for the delay. First up, thanks heaps for looking into this! It's
very exciting.

You're welcome, thanks for looking into this!

What monkey-patching are you referring to? The code in
GLibEventLoopPolicy.__init__? If so, I agree that this is not ideal, but
that's easy enough to fix up - we just need to provide implementations of
those methods. The gross monkey-patching is (I think, it's from before I
took over the project) used as a shortcut to avoid implementing them
ourselves, which can be done completely separately to your work. - We're
overriding private methods quite a lot, which I don't consider to be
well-defined. How much work do you think it would be to stop doing this,
and instead override the public methods that call them?

Actually the submitted code does exactly that:

1. GLibBaseEventLoop only overrides public and "protected" methods without implementation in base_events.BaseEventLoop. In base_events each of those protected methods (such as _make_socket_transport) just contains a single raise NotImplementedError() and a comment on what their actual implementation should be doing, so I believe these interfaces to be fully stable for subclass implementation use.
2. GLibEventLoop then override several public methods for an optimized implementation based on an actual glib.MainLoop.

If it's too much work/pain, that's okay, it just means we'll have to write a lot
more tests and get more serious about our automated testing, which is
something I've wanted to do for a while now anyway.

I'd recommend adding in the integration tests of serious asyncio event loop consumers (such as aiohttp or aiodns, maybe aiopg for UDS testing?) to the test suite as well. So that differences in edge-case-handling in new patches can be quickly be detected.

You say that TLS does not work with Python 3.4 and below, but later on you
mention that you've not tested it with anything older than 3.5.2. Is this referring
to the use of the new SSL API?

Yes, the current code uses ssl.MemoryBIO, which is only available in python 3.5+, and doesn't have any of the necessary fallback code for python 3.4 (or below) on Unix.

Where has gbulb/transports.py come from? This is a massive amount of
code that is completely untested. A non-trivial portion of it seems to be
copy-pasted from proactor_events.py, which raises licensing concerns and
may cause maintenance headaches.

I'm sad about this as well, but implementing the official interfaces (as outline above) requires one to either (a) implement your own transport implementations or (b) reuse one of the official implementations. The problem with (b) being, that both of the official implementations rely on private implementation details of their respective event loops to implement their transport functionally. (Which is a real pity because the Window proactor_events event loop can be transformed into a generic transport implementation, that doesn't depend on any specific event loop internals, without too much hassle – and, as you have correctly identified, that is exactly what I did.)

Regarding licensing issues: I'm still not sure whether asyncio is made available under the terms of the Apache License 2.0 or under the terms of the PSF License. Either way however adding proper attribution and "a brief summary of the changes made to Python" (PSF License section 3) will suffice to work around any problems that may be caused by this.

Regarding maintenance: It's hard to say how this will play out, but it is somewhat unlikely that any major changes to this code will be required anything soon. After all, all used APIs (event loop, sockets, public interfaces) are stable and the concept of transports itself has had some long-time testing in the twisted project before asyncio was even born (note: asyncio is basically a reimplementation of of twisted's event reactor concept).

As I said I'm not particularly happy about this either, but I believe it to be the lesser evil.

asyncio.events._set_running_loop and asyncio.events._get_running_loop
calls are sometimes, but not always, guarded with existence checks beforehand.
Please fix up the ones that don't have such checks.

Could you please point me to the place in GLibEventLoop.run() where I forgot to guard the calls. AFAIK everything is correctly guarded (or the code wouldn't have been able to run of python 2.5.2).

Please ensure there are no tabs in the code. There are many in
gbulb/transports.py, for example.

Will do. 😿

The implementation of GLibBaseEventLoop.select is pretty awesome!

Thanks!

In future, if you could use smaller, more clearly separated commit messages,
that would be good

I will, this was just the big "getting it off the ground" commit that contained several days of trying and rewriting things to produce something that would actually work.

I think most of this code is not necessarily Windows
specific, so at the moment I'm having quite a bit of difficulty finding the
changes that are directly necessary for Windows support.

Yes, but almost all of the code in glib_events.py is now totally GLib-specific – leveraging GLib's cross-platform support, where the code previously attempted to reduce GLib to a mere select() wrapper (which ofc fits the use-case as part of SelectorEventLoop pretty well, but not so much the cross-platform usage). It is true however that it would probably have been possible to implement all of this as part of the previous architecture, but the it became quite hard to wrap one's head around which parts of the public API interact with GLib in which way – I certainly failed when I tried to initially do just that…

All of that said though, I think this is pretty awesome! I've created a new
branch named windows-support, if you can change your base branch to that
one instead, I can start merging in changes without fear of breaking
master. Once we're satisfied with the quality, I can merge it into master.

Thanks, will do!

Before I can start working on adding support on UDP, UDS, Pipes, subprocesses, … we need to settle the question on how to proceed with transports.py, because all of these will probably require at least minor additions to that module in order to work. Should I just continue adding (and if it seems necessary/useful also copying) stuff there in order to implement the desired functionality? I'm definitely open to any alternatives for this!

[ntninja]

Ok, I'm (finally) done with the implementation now! All mentioned issues have been addressed (using the current transport approach)!

Currently there is one test failure left on Unix:

The TestBaseGLibEventLoop.test_call_soon_priority checks whether a writer attached to a pipe that completes a future is rescheduled before the completion callback of the future is invoked. I know that I broke this test when fixing an issue with sock_accept on Windows (see commit 427f5a2), so I checked whether this test would hold on the asyncio reference implementation of the event loop for Linux (Unix event loop with epoll selector) and it turns out that the writer callback is scheduled twice there as well!
In other words: I contest that this test is actually testing any defined asyncio event loop behaviour and think that it can and should therefor be removed because it simply does not apply anymore.

That's all from me for now! 😀 Waiting for your review of the (currently) final codebase!

[ntninja]

Oh and I should add that I wrote this set of basic tests while hunting for bugs in the new codebase:
http://ipfsbin.xyz/#QmXmgW6i5KqMvkLaXP92Dh4GidZqnRzuG8qgaSPJ6DzdBz

Reference output:

Read pipe opened <gbulb.transports.PipeReadTransport object at 0x7fef2bfc1ba8>
Write pipe opened <gbulb.transports.PipeWriteTransport object at 0x7fef2bfcb7f0>
Write pipe closed None
Read pipe data b'Halleluja!'
Read pipe EOF
Read pipe closed None
UDP Endpoint bound
Running UDP endpoint: <gbulb.transports.DatagramTransport object at 0x7fef27919d68>
UDP received: 'Hello World' (from ('::1', 8888, 0, 0))
UDP transport closed
UDP Endpoint lost None
TCP Serving on ('::1', 8888, 0, 0)
Child returned: (b'Maybe now?', None)
28403
TCP Client connection made <gbulb.transports.SocketTransport object at 0x7fef21662860>
TCP Server connection from ('::1', 35054, 0, 0)
TCP Server data received: "b'GET / HTTP/1.0\\r\\n\\r\\n'"
TCP Server data sent: "b'GET / HTTP/1.0\\r\\n\\r\\n'"
TCP Server closed client socket
TCP Client got data b'GET / HTTP/1.0\r\n\r\n'
TCP Client connection lost None

Maybe that's useful for additional unit tests?

[nhoad]

This is awesome!! Fantastic work! Remarks/questions:

• We're fine to keep using gbulb.transports and whatnot as we are now, IMO. It works, so I think we should stick with it.
• I see we have some code for fixing Python 3.4.3, so I'll add that to the build bot.
• I agree about call_soon_priority, now that it's working as intended I think we're fine.
• When I load the link up for the ipfsbin.xyz, I get a blank page, so I'm not sure what the tests actually look like. I definitely think we want them though! Now that we're a real implementation, I think testing as much as possible is a good idea, given we're not relying on SelectorEventLoop anymore :)
• How did you manage to trick aiohttp into using gbulb?
• I'm not sure how I feel about using third party code for integration tests. The idea seems reasonably sound, except that as you've also discovered, aiohttp had some tests that were incorrect, so I'd be slightly worried about that making things difficult.

As an aside, I've got Linux-based CI for 3.4, 3.5 and 3.6 going now. I'll check out appveyor over the weekend and see how I go with that, and then I think I'll be much more comfortable merging this in :) thank you again for all this work! This is above and beyond what I expected anyone to do.

[ntninja]

🎉 So we're basically done? 🎉

Regarding you last couple of points:

• Yup, the bin is broken in deed, let's try something less experimental instead: http://pastebin.com/ee5KM55V 😉
• "Tricking" aiohttp into using gbulb was simple, but you'll probably be disappointed about the level of primitiveness of the hack 😛:
  1. Clone the aiohttp repository and open its test directory
  2. Add a symbolic link (inside the test directory) to gbulb's source files (unless you actually installed gbulb system-wide or in a virtualenv of course)
  3. Open conftest.py and add import gbulb and gbulb.install() as the first two lines, save
  4. Run py.test-3 as usual!
• I do agree with you regarding the possible problems with using third-party test suites, however the deciding factor on whether it's a good idea to do this or not is how co-operative upstream (of the test suite) is in ensuring that they will only use documented event loop features in their tests. And how likely they are to disregard any issues we may encounter as "doesn't happen with the reference implementation – wontfix". Using their test suite without any kind of prior agreement could indeed result in unexpected and frustrating problems, so asking them nicely about this is IMHO the first logical step in (possibly) making this kind of co-operation possible.
• Also I should finally get those aiohttp issues I previously found reported… That would help with asserting their reaction to these kinds of issues as well…

[nhoad]

I think so!

These tests look useful, I'm happy to transform these into proper tests. Your trickery was quite obvious in retrospect, I should have thought of conftest! :)

How have you been building and testing on Windows, by the way? It's been about ten years since I've built any software on Windows, so this is all completely unfamiliar to me. MSYS2 seems to be the suggested environment for building GTK3 packages on Windows, but it doesn't seem to be a sensible way to run Python, particularly given sys.platform is "msys". I haven't even made it to the point where PyGI is a possibility, so I'm curious as to what I'm missing :)

[ntninja]

How have you been building and testing on Windows, by the way?

I'll hereby confess that I haven't built a single Windows binary on Windows in my entire life 😸
The entire development of this code happened on Linux and testing was done in a Windows 7 VM with MSYS2 & MinGW32. Setup is quite simple:

1. Download installer from website
2. Follow the installation steps on the website (run installer, installer, start shell, update & upgrade)
3. Restart shell
4. Run pacman -S mingw-w64-i686-python3 mingw-w64-i686-python3-gobject mingw-w64-i686-gtk3 (package list may be incomplete…) to install the required packages to deps
5. Start python by simply typing python3

I also found the official msys2 shell to be unbearably slow and causing issues with interactive terminal mode for some reason, so I switched to good old cmd.exe instead. This works without issues, you just need to export the correct %PATH% after launching it, by typing:

path C:\msys32\mingw32\bin;C:\msys32\usr\sbin;C:\msys32\usr\bin;%PATH%

(WITHOUT any extra quotes!) before continuing as if it were a Linux shell.

Important clearification: Apps compiled with MinGW run as normal Windows apps on the win32-API and can be used outside of the msys2 install. In Python this means that the interpreter will report sys.platform == "win32" and GLib will use MsgWaitForMultipleObjectsEx for its I/O polling. Using the MSYS2 installer is simply a way to greatly simply the install procedure (at the cost of some disk space 😉).

.

Alternatively you can try finding a Python 3.4.x installer somewhere online and install the PyGObject for Windows suite on top. (As far as I can tell from the changelog their somewhat reluctant to break Windows XP support and supporting Python 3.5 [which doesn't support Windows XP anymore] seems to force that for some reason.)
I must confess that I did not actually test the code using Python 3.4 on Windows, and that I in particular did not go even near Windows XP with this. Since the Windows version for Python 3.4 will not support TLS and was never before supported, I suppose it really doesn't matter in the end.

[stuaxo]

Gtk dropped support for Windows XP, so you don't need to worry about it.

Something to be aware of is if your package contains compiled elements then you can't mix MSys and VC++ compiled python + libraries.

(I don't think gbulb does).

Python + Windows is not the best - a lot of people have switched to Conda which makes a lot of these things easier, but last time I looked it didn't have all the Gtk libraries that I needed.

[nhoad]

Sorry for the lack of communication on my part! I've been swamped with a bunch of things lately. I should have some time this weekend to finally get around to Windows testing.

[nhoad]

This test breakage is in test_call_soon_priority, which we talked about, so I'm happy to merge this in and then remove the bogus test.

While I'm here, I finally tested things on Windows and it worked quite well! Thank you so much for this work, it's fantastic.

[gbtami]

@Alexander255 can you take a look at issue #24 please!