Fixes #7758

[yglukhov]

No description provided.

[dom96]

Looks good and will merge once tests pass.

[cheatfate]

This fix is incredibly wrong.

• Because of sleep(), all tasks which must be completed via processPendingCallbacks() immediately, will wait for sleep timeout.
• Fix is only made for Unix, and there no fix for Windows.

[yglukhov]

Added the same fix for windows.

[yglukhov]

@cheatfate, the problem you described indeed exists (for a long time already), however it is irrelevant to this fix.

[cheatfate]

Immediately completed tasks, which will use callSoon() will be not processed smoothly if there timer present. This tasks will be completed only after proposed sleep call finishes.

[yglukhov]

@cheatfate, you could write a test case for that, and I could prove you the problem existed long before, and still exists.

[yglukhov]

@cheatfate, ok don't bother. Here's a proof:

import asynchttpserver, asyncdispatch, times

let s = newAsyncHttpServer()
proc handleClient(request: Request) {.async.} = discard
asyncCheck s.serve(Port(8080), handleClient)

let wait = sleepAsync(200)
waitFor(wait) # Wait a bit to let the server initialize

var startTime, endTime: float

let f = newFuture[void]()
f.callback = proc() =
  endTime = epochTime()
  echo "The message! It took us ", endTime - startTime, " seconds to display it"
f.complete()

echo "now you should immediately see the message..."
startTime = epochTime()
poll(2000)

Outputs:

now you should immediately see the message...
The message! It took us 2.000142097473145 seconds to display it

This code works equally the same with my pr or without it.

EDIT: As a side note, this issue can be fixed by merging #6614...

[cheatfate]

Your code is working because of behavior described in #7193 (immediately completed Futures got completed immediately at any cost). If #7193 got fixed, and your PR will be accepted, then new bug with incredible timeouts will be introduced.

[yglukhov]

@cheatfate, you're probably right, but nevertheless this is out of the scope of this pr.

[cheatfate]

@yglukhov could you please mark your sleeps with comment about review this code after #7193 will be fixed.

[yglukhov]

@cheatfate, can you fix #7193? If yes, then please do. Otherwise I don't see how my sleeps are related to that more than current selectInto branch.

[cheatfate]

I can't fix it easily. The easiest way to fix it is to revert all changes to callSoon, because original callSoon don't have such problems. Also whole poll() function must be reworked.

If #7193 got fixed, then immediately completed Futures, e.g. tasks performing their job without any continuation will finish via callSoon(). And because callSoon() is working with loop.callbacks list, it will be processed after your ugly sleep().

So this simple example will finish after 10 seconds, but all other async world will finish it immediately:

import asyncdispatch

proc task1() {.async.}
  await sleepAsync(10000)

proc task2() {.async.} =
  echo "This procedure must be completed immediately"

when isMainModule:
  asyncCheck task1()
  waitFor task2()

[dom96]

So this simple example will finish after 10 seconds, but all other async world will finish it immediately:

Completes immediately for me. waitFor only calls poll while task2's future is incomplete.

[cheatfate]

@dom96 it completes immediately now, because #7193 is not resolved/fixed.
Sample will finish after 10 seconds if and only if

• Asynchronous scheduler behaviour #7193 resolved/fixed.
• Fixes #7758 #7759 accepted/merged.

1. After asyncCheck task1() one timer will be added to dispatcher.timers.
2. After call to task2(), internally will be called callSoon(), which will add callback to dispatcher.callbacks.
3. Because task2() is not yet finished poll() will be called by waitFor.
4. Because there no registered handles in dispatcher this branch will be used: https://github.com/yglukhov/Nim/blob/f7d837d47ca1b8bbc83b1d6df9b8d71c98b677d1/lib/pure/asyncdispatch.nim#L346-L347 or https://github.com/yglukhov/Nim/blob/f7d837d47ca1b8bbc83b1d6df9b8d71c98b677d1/lib/pure/asyncdispatch.nim#L1275-L1276, so sleep will be called with last timer and starts waiting for 10 seconds.
5. Only after this 10 seconds waiting processPendingCallbacks() will finish task2().

[yglukhov]

I can see 2 simple ways of solving your problem.

1. Always processPendingCallbacks at the beginning of poll and at the end of it.
2. Reduce timeout to 0 (in other words, skip select or sleep), if there are pending callbacks.

[Araq]

This was fixed by @dom96 differently. Closing.