armanbilge/epollcat

`EpollExecutorScheduler` hangs if timeout < 0

Closed this issue · 6 comments

It's not a very nice user input, but we can be graceful about it.

I started a fix for this here, but now I think this is something I should fix in Cats Effect actually. This is not arbitrary user input we are receiving, but rather something calculated by the runtime.

https://github.com/armanbilge/cats-effect/blob/5d11fe96c446300add6a34d6e3f2a109301cfc74/core/native/src/main/scala/cats/effect/unsafe/PollingExecutorScheduler.scala#L128-L136

FTR, b6ac125 was the fix I initially considered here.

I looked at b6ac125. If negative timeout can not be a flat out "Invalid Arg" error, then
why is b6ac125 not still relevant?

If somebody did it once, it will almost surely happen again, especially if there is
any chance at all of user (application caller) providing the argument.

Sorry if I missed something....

Good question, sorry for lack of context.

If somebody did it once, it will almost surely happen again, especially if there is
any chance at all of user (application caller) providing the argument.

This is exactly why I changed my mind: the poll method is protected. Users should not be calling it; it is only invoked by the Cats Effect runtime. So the correct fix is for Cats Effect to never call this method with invalid arguments.

So the correct fix is for Cats Effect to never call this method with invalid arguments.

Thank you for explaining. There is always a question "contract" and how deep argument
checking goes. Agreed "correct" fix is Cats Effect. Given your description of the
restricted calling environment, I now understand the lack of argument checking in
the callee.