glib_selector: Only ever iterate the mainloop once #9
Conversation
|
I think this is a much better approach to fix this than #4, #7 and #8 are. No need to override more methods on the python side and I think it is reasonable to return to python after every GLib mainloop iteration. Really, conceptually we just run a GMainContext using the python mainloop (in a somewhat backward way, but it is reasonable). |
The GLib mainloop only runs at 1ms accuracy, as such, we need to lower the accuracy as seen by the python mainloop in order to not busy wait for timeouts to be scheduled.
| self._fd_to_tag = {} | ||
| self._fd_to_events = {} | ||
| self._main_loop = main_loop | ||
| self._iteration_timeout = 0 |
There was a problem hiding this comment.
Nitpick, I would drop the leading '_' or set it through a function, honoring the python convention of 'private variables'.
There was a problem hiding this comment.
I think it is completely normal to add a leading _ for internal (module private) variables. Or is that for private to subclasses only?
There was a problem hiding this comment.
Not explicitly defined in PEP8 however as in lots of (if not all) languages where there exists a strict private/protected keyword, this applies to the context of definition. Thus in this case the _SelectorSource class, which on its turn can be and is marked as internal within the module scope (which will exclude it and all its members from a default import).
On line 83 and 85 the _iteration_timeout variable is accessed from outside of the class scope, hence implying it should actually be public.
But again, it's just a nitpick (and probably something contested, which you get with conventions)
As it is right now, any python code that runs may modify the timeout that would be passed into the select() call. As such, we can only run a single mainloop iteration before the select() call needs to be restarted. Also, we cannot use g_source_set_ready_time, because GLib will always do a full mainloop iteration afterwards to ensure that all sources had a chance to dispatch. This is easy to work around though as we can use the prepare callback to pass the required timeout from python into the GLib main loop code. Note that with this we end up iterating the main context but we never actually run a GLib mainloop. Fixes: jhenstridge#3
This test seems to be new and relies on sigalarm interrupting the select. But that isn't really a valid assumption with GLib driving it currently (i.e. it would need extra handling).
As it is right now, any python code that runs may modify the timeout
that would be passed into the select() call. As such, we can only run a
single mainloop iteration before the select() call needs to be
restarted.
Also, we cannot use g_source_set_ready_time, because GLib will always
do a full mainloop iteration afterwards to ensure that all sources had a
chance to dispatch.
This is easy to work around though as we can use the prepare callback to
pass the required timeout from python into the GLib main loop code.
Note that with this we end up iterating the main context but we never
actually run a GLib mainloop.
Fixes: #3