-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Clean asynchronous stop_callback implementation #1523
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
Conversation
@@ -897,7 +903,7 @@ def _select(sockets): | |||
read_allowed_exceptions = (PcapTimeoutElapsed,) | |||
|
|||
def _select(sockets): | |||
return sockets | |||
return select_objects(sockets, remain=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: separate
WINDOWS and started_callback
==> use select_objectsWINDOWS and not started_callback
==> previous behavior: dont use select_objects
@gpotter2 what is the status of this PR? |
Quite unknown :/ I’m still not sure wether this was a good idea at all I’ll run some benchmarks and try to make it work with the recent select overhaul, to see what it becomes :/ |
This is a VERY useful feature, implemented in a way consistent with how it has been done in C since at least the mid 90's, and thus well battle proven. I would REALLY like to see this coming out in a release. The only comment I have is that if you return something more resembling an object, rather than a mere stop_... function, you can easily in the future add additional reasons for communicating with the sniff function without breaking backwards compatibility. So instead of sending the "stop_callback" function to "started_callback", send a communication structure allowing the user to call "commstruct.stop_sniffing()". That way, in the future you can add other methods, like "commstruct.extend_timeout()" or whatever. Other changes needed to accomodate new features (such as not merely clearing the select list when objpipe is written to) will be well hidden behind the interface. I am a huge fan and really appreciate your work. I hope to see this feature in a not too distant release. |
Good point. I think the best option would be to implement a |
This PR:
sniff
I will add the unit tests if we agree on the PR