Gevent: Extending monkeypatch to builtin open, os.fdopen, io.open etc

Created on 8 Jan 2018  ·  5Comments  ·  Source: gevent/gevent

I was surprised to find that even after monkeypatching everything under the Sun, after creating the pipes from os.pipe the os.fdopen or open or io.open did not return cooperative file objects. Why?

The simplest idiom of running a select loop that is notified to unblock by listening on read end of pipe, unblocks and adds and removes FDs from notifications lists cannot be realized in such a fashion.

It seems to me that if we want to turn a non-cooperative code cooperative, it might be essential to simply turn all IO cooperative, which means patching all file-creating mechanisms or maybe even such low-level mechanisms as pipe, pipe2 etc.

Based on whether the file being opened is actually a socket/pipe or storage, monkey code can respectively pick POSIX- or threadpool-based cooperative files.

Is this an oversight or my lack of understanding and there are fundamental deeper problems with such thorough patching?

Question

All 5 comments

There are some fundamental issues with monkey-patching at those levels.

Regular files are easy to explain. Using FileObjectThread, which is required to make regular files cooperative, adds non-trivial overhead. That excludes open and io.open for regular files. Adding the stat calls to try to detect that the opened path is a unix socket or something else that can usefully have O_NONBLOCK set on it seems like a lot of overhead for very little gain---most of the time that you're going to do something like that you're just going to ask for the fileno() and turn around and manually make it non-blocking to select on it anyway. (Plus, there are compatibilty concerns: there is code in the wild that does things like if isinstance(f, file):; I seem to remember something like that in ZEO.)

The low-level calls like os.pipe are somewhat more difficult to explain, and it basically comes down to compatibility. For that to work, the patch would have to put the fileno in non-blocking mode before returning it. Because it's returning an integer, there's no other way to customize the behaviour (returning some kind of wrapper object is right out; filenos get passed to C, and C code often assumes that they are legitimate int objects, not even subclasses---#1066 is a partial example in the CPython implementation itself) There are parts of the standard library that will get very unhappy very quickly if pipe filenos it thinks are blocking are not---multiprocessing.queue.Queue being one example that comes immediately to mind. User code is also likely to be affected. One could go a step further and also patch os.read/os.write to be nb_read and nb_write, but that's beginning to get pretty invasive---and what if user code actually wanted to make non-blocking reads on non-blocking filenos? Because they're just integers, we have no way to know what the intent is without some sort of very expensive mapping that keeps track of which filenos we have implicitly turned non-blocking and which the user has explicitly turned non-blacking.

Historically, gevent tries to monkey-patch at the highest possible levels and leave as many low-level details in place as possible for these kinds of reason. Patches in os especially are very carefully considered and have a very high bar to clear. There's some more discussion about that in #622.

Always love how thorough your explanations are.

Thank you! Is there more to discuss here or can it be closed?

You're welcome :smile:

I guess I'm not sure I buy this argument.

The "dream" of monkeypatching is to allow one to turn existing or to build new blocking multi-threaded code with easy-to-follow imperative pattern, while executing it in a cooperative single thread without resorting to any code modifications or sprinkling async and away everywhere. To me it's an incredibly powerful concept - turning blocking threaded code into non-blocking loop code without any code changes. But, unfortunately, for that dream to be realized it has to work for all code that may block (which in Python are sync primitives, I/O and native libs that make whatever they do opaque and unpatchable) or it doesn't work at all.

Both Python async and gevent have overheads over a single-threaded blocking code (loop, coroutines and futures vs greenlets and stack copying on context switch respectively). However the pros of explicit or implicit asynchronicity outweighs those of writing (pseudo-)threaded code or highly advanced customized non-blocking code using OS-specific primitives.

Replacing open, io.open or os.fdopen with one that stats once on open doesn't seem like terrible overhead (generally same file is not opened in a tight loop repeatedly and it's only done once per file anyway). So overhead is already O(1). More importantly, this is an argument for "code that has slight increase in overhead while working under gevent monkey OOB" vs "code that renders gevent loop useless because it blocks on IO". I'm also quite sure that we can stick file into mro somewhere to make sure isinstance(f, file) returns True.

As far as os.pipe and other FD-generating calls, I think there is even less problem with them, frankly, than with higher level primitives. If we take a pipe or a socket fd, gevent needs to make them non-blocking so that the code cooperates. If that fd is turned into a file primitive before use, then (assuming the latter is patched) it's a cooperative gevent file. If it's used as int then with patched os.read/write it is still total-order correct and behaves cooperatively. The only time this should not work if patched blocking code requires/checks the fd to be blocking, i.e. the code literally goes "this FD is non-blocking, turn it blocking and break if it's not". I somehow highly doubt that this code 1) exists 2) would be used with gevent.

As far as anything in multiprocessing breaking, it already requires a major rewrite as I understand as it doesn't work if threading is patched.

I would be interested in trying to bring this additional patching in to see if can achieve virtually complete drop-in replacement for any code that makes sense.

I too find the counter arguments unconvincing 😄

Rather than rebut them in detail point by point tonight (things like: opening socket files is platform dependent anyway, if you want that in an independent way try socketpair or localhost sockets, in which case everything just works already; there are more problem with os.pipe than there are higher level APIs becase there is no extra state associated with an int and filenos are passed around and used as if they are blocking frequently, often by C APIs---I gave one example, another one is readline, another one is pyOpenSSL, etc---and yes, those get used in monkey-patched applications successfully; the definition of "code that makes sense" is extremely broad and I'm not interested in narrowing it, although gevent's primary use case is networking, to quote the documentation) I'll say this:

I'm really not interested right now in dealing with the bug reports that I feel certain would flood in were gevent to try this (or worse, would discourage the use of gevent at all; every incompatibility turns people away even if I promptly find a fix) 🤷‍♂️ In any event, something that aggressive is definitely out of the scope for 1.3 and probably 1.x; maybe it can be a 2.0 feature someday. Or, with a little more work defining the way in which gevent monkey-patches (it's getting more stable and extensible with each release), to steal the standard library's approach, that could be an optional "plug-in" that's initially distributed via PyPI in order to gauge interest and compatibility before including it once it's reached stability. 😄

Anyway, this has been an interesting conversation. I'm glad it's in the archives.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

TylerMills picture TylerMills  ·  10Comments

Michael-F-Ellis picture Michael-F-Ellis  ·  4Comments

2trc picture 2trc  ·  7Comments

MuslimBeibytuly picture MuslimBeibytuly  ·  3Comments

ClericPy picture ClericPy  ·  6Comments