Fix Unix Poll Method (Issue 82) #83
Conversation
Fixes Unix Poll Method to use eventmasks so that on MacOS 10.14.6 poll does not return True when the Read pipe channel reports it ready for output.
Reworked Poll Eventmask to have a wider range of read only event flags.
|
Hello, Thanks for providing this. I am on vacation right now, I'll come back to this once back. |
pebble/pool/channel.py
Outdated
| poll.register(self.reader, READ_ONLY_EVENTMASK) | ||
|
|
||
| # Convert from Seconds to Milliseconds | ||
| timeout = timeout * 1000 if timeout else timeout |
There was a problem hiding this comment.
It is usually an antipattern to test for None objects treating them as booleans:
https://www.python.org/dev/peps/pep-0008/#programming-recommendations
In this case, for example, if by chance the unix_poll() function would be given a large string, you would end up multiplying it by a factor of 1000 most likely sending the program to OOM.
It is always recommended to test explicitly against None values.
timeout = timeout * 1000 if timeout is not None else NoneOr more simply in this case:
if timeout is not None:
timeout *= MILLISECONDS
...
MILLISECONDS = 1000
|
Thanks for the heads up! I have swapped the None anti-pattern. I did not add any checks for strings in this commit, so I suppose there is still a risk of large string issues. I am happy to refactor in a check to see if timeout is a number (int or float) if you want added safety. |
|
No need to check for the input type. After all, Python is using dynamic typing on purpose. Thanks for your contribution! I will bake a release this WE. |
For #82
Fixes Unix Poll Method to use eventmasks so that on MacOS 10.14.6 poll does not return True when the Read pipe channel reports it ready for output.