Nim icon indicating copy to clipboard operation
Nim copied to clipboard

std/asyncfile is not async. Documentation don't mention it

Open Alogani opened this issue 1 year ago • 1 comments

Description

Hello,

std/asyncfile implementation is not truly async because it doesn't use selectors (at least on Unix. I don't know on Windows). The documentation don't mention it, resulting in deceptive behaviour

This is not a problem for regular files. But it's problematic for fifo, pipes, etc. For example, creating an asyncfile from stdin won't have the expected behaviour, because the call will be blocking the main thread.

Nim Version

2.0.2

Current Output

No response

Expected Output

No response

Possible Solution

The solution is to modify the current implementation to use for example addRead before reading asyncfile. For epoll, using addRead on regular file will raise an error.

Here are some proposal from some of my own library :

proc readSelect(self: AsyncFile): Future[void] =
    result = self.readListener.wait()
    if not self.pollable or self.readListener.isListening():
        return
    if bool(self.cancelled):
        self.readListener.trigger()
        return
    self.readListener.clear()
    proc cb(fd: AsyncFD): bool {.closure gcsafe.} =
        self.readListener.trigger()
        true
    AsyncFD(self.fd).addRead(cb)
## Or simpler, but could result in multiple callbacks for same AsyncFd :
proc readSelect(self: AsyncFile): Future[void] =
    var fut = newFuture[void]()
    proc cb(fd: AsyncFD): bool {.closure gcsafe.} =
        var fut.complete
        true
    AsyncFD(self.fd).addRead(cb)
    return fut
## Solution on Unix only
proc isPollable(fd: cint): bool =
    ## EPOLL will throw error on regular file and /dev/null (warning: /dev/null not checked)
    ## Solution: no async on regular file
    var stat: Stat
    discard fstat(fd, stat)
    not S_ISREG(stat.st_mode)

Additional Information

No response

Alogani avatar Mar 29 '24 20:03 Alogani

On Windows it is async.

Araq avatar Mar 29 '24 21:03 Araq