Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to wait for POLLPRI #463

Closed
wants to merge 2 commits into from
Closed

Conversation

smurfix
Copy link
Contributor

@smurfix smurfix commented Feb 28, 2018

This extension is required for some interesting things. Personally I need it for accessing GPIO pins (specifically, epoll()ing for changing inputs).

I still have to figure out how to reliably test this, given that the TCP URGent flag is less than well supported these days, Travis is unlikely to run on a machine with two connected GPIO pins, and I haven't done anything with PTYs in a decade.

This function is only available for epoll. It uses the EPOLLPRI mode to
trigger on an "exceptional condition", whatever that is with respect to the
given file descriptor.

Among other uses, this flag can be used to wait for changes on GPIO inputs,
or for mode changes of a PTY slave device (when used on the master).
That would give us false positives.
@codecov
Copy link

codecov bot commented Feb 28, 2018

Codecov Report

Merging #463 into master will decrease coverage by 0.29%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #463     +/-   ##
=========================================
- Coverage   99.26%   98.97%   -0.3%     
=========================================
  Files          89       89             
  Lines       10403    13474   +3071     
  Branches      718     1517    +799     
=========================================
+ Hits        10327    13336   +3009     
- Misses         58      110     +52     
- Partials       18       28     +10
Impacted Files Coverage Δ
trio/hazmat.py 100% <ø> (ø) ⬆️
trio/_core/_io_epoll.py 91.01% <50%> (-8.99%) ⬇️
trio/_core/_io_windows.py 69.54% <0%> (-6.34%) ⬇️
trio/_util.py 96.23% <0%> (-3.77%) ⬇️
trio/_highlevel_socket.py 98.68% <0%> (-1.32%) ⬇️
trio/tests/test_socket.py 99.4% <0%> (-0.6%) ⬇️
trio/_core/_run.py 99.92% <0%> (-0.08%) ⬇️
trio/_highlevel_generic.py 100% <0%> (ø) ⬆️
trio/_sync.py 100% <0%> (ø) ⬆️
trio/tests/test_timeouts.py 100% <0%> (ø) ⬆️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e0b92b...6c41fb4. Read the comment docs.

@njsmith
Copy link
Member

njsmith commented Mar 1, 2018

The issue for this is #61.

Looking at the docs linked there, I see that the described gpio API is now considered obsolete and broken. Out of curiosity, does the new "character device API" also use POLLPRI? (I realize it will be a while before everyone has kernels with the new API, and that there are other possible use cases for this, but I find it never hurts to know more about how things are used :-).)

A fun thing to deal with is that on Windows when a non-blocking connect fails, this is reported by marking the socket as "priority". Everyone else uses "readable" for this. Currently we hack around this the same way that some other libraries do, by making the windows wait_socket_readable function actually wait for either read or priority. If we wanted to support wait_socket_priority there as well then we'd need to figure out how let them coordinate.

I guess one option would be to remove the hack from the io layer, and instead have our connect method wait for both read and priority. With the current io API, this would require that connect open a nursery to call both wait functions simultaneously. That feels a little weird, but I guess would be ok? Are there workloads that care about getting maximum speed out of connect on Windows? Another option to consider would be redoing the API to allow a task to wait for several events at a time, like wait_io_ready(flags). (Some connection here to #242, though that gets into much more complicated issues.)

Another Windows-related thing to consider is that if we switch to iocp-only (#52), then we won't have a way to wait for priority at all on that platform. Though fortunately we won't need to, either, since the iocp version of connect works differently.

@njsmith
Copy link
Member

njsmith commented Apr 4, 2018

Github is telling me you just pushed some strange things here?

@smurfix
Copy link
Contributor Author

smurfix commented Apr 4, 2018

Gaah. Sorry, I'll undo this nonsense.

@njsmith
Copy link
Member

njsmith commented Jul 31, 2018

This is very stale, so going to close for now... feel free to re-open if you want to pick it up again.

@njsmith njsmith closed this Jul 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants