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

Fixes crashes when asyncdispatch.adjustTimeout returns a negative value. #11231

Merged
merged 1 commit into from May 15, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -211,11 +211,10 @@ proc processPendingCallbacks(p: PDispatcherBase; didSomeWork: var bool) =
didSomeWork = true

proc adjustTimeout(pollTimeout: int, nextTimer: Option[int]): int {.inline.} =
if nextTimer.isNone():
if nextTimer.isNone() or pollTimeout == -1:
return pollTimeout

result = nextTimer.get()
if pollTimeout == -1: return
result = max(nextTimer.get(), 0)
result = min(pollTimeout, result)

proc callSoon*(cbproc: proc () {.gcsafe.}) {.gcsafe.}
@@ -378,6 +378,8 @@ proc selectInto*[T](s: Selector[T], timeout: int,
if maxres > len(results):
maxres = len(results)

verifySelectParams(timeout)

let count = epoll_wait(s.epollFD, addr(resTable[0]), maxres.cint,
timeout.cint)
if count < 0:
@@ -44,8 +44,8 @@ elif defined(netbsd) or defined(openbsd):
when hasThreadSupport:
type
SelectorImpl[T] = object
kqFD : cint
maxFD : int
kqFD: cint
maxFD: int
changes: ptr SharedArray[KEvent]
fds: ptr SharedArray[SelectorKey[T]]
count: int
@@ -57,8 +57,8 @@ when hasThreadSupport:
else:
type
SelectorImpl[T] = object
kqFD : cint
maxFD : int
kqFD: cint
maxFD: int
changes: seq[KEvent]
fds: seq[SelectorKey[T]]
count: int
@@ -447,6 +447,8 @@ proc selectInto*[T](s: Selector[T], timeout: int,
ptv = addr tv
maxres = MAX_KQUEUE_EVENTS

verifySelectParams(timeout)

if timeout != -1:
if timeout >= 1000:
tv.tv_sec = posix.Time(timeout div 1_000)
@@ -214,6 +214,8 @@ proc selectInto*[T](s: Selector[T], timeout: int,
if maxres > len(results):
maxres = len(results)

verifySelectParams(timeout)

s.withPollLock():
let count = posix.poll(addr(s.pollfds[0]), Tnfds(s.pollcnt), timeout)
if count < 0:
@@ -309,6 +309,8 @@ proc selectInto*[T](s: Selector[T], timeout: int,
var ptv = addr tv
var rset, wset, eset: FdSet

verifySelectParams(timeout)

if timeout != -1:
when defined(genode):
tv.tv_sec = Time(timeout div 1_000)
@@ -314,6 +314,14 @@ else:
key.events = {}
key.data = empty

proc verifySelectParams(timeout: int) =
# Timeout of -1 means: wait forever
# Anything higher is the time to wait in miliseconds.
if timeout < -1:

This comment has been minimized.

Copy link
@Araq

Araq May 14, 2019

Member

use doAssert(timeout >= -1) instead, programming bugs are not exceptions to be handled.

This comment has been minimized.

Copy link
@dom96

dom96 May 15, 2019

Author Member

Then why do we even have a ValueError exception? Isn't that its purpose?

This comment has been minimized.

Copy link
@Araq

Araq May 15, 2019

Member

Regex parsing can fail because the user's input might not be a valid regex --> exception, programmer cannot pass the right value to timeout --> a bug, not an exception.

This comment has been minimized.

Copy link
@dom96

dom96 May 15, 2019

Author Member

Makes sense.

raise newException(
ValueError, "Cannot select with a negative value, got " & $timeout
)

when defined(linux):
include ioselects/ioselectors_epoll
elif bsdPlatform:
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.