diff --git a/src/sys/handles.nim b/src/sys/handles.nim index 9bba986..0782f8b 100644 --- a/src/sys/handles.nim +++ b/src/sys/handles.nim @@ -125,12 +125,29 @@ when false: # XXX: T should be a concept, but those don't work atm type - Handle*[T] {.requiresInit.} = object + Handle*[T] = object ## An object used to associate a handle with a lifetime. - # Walkaround for nim-lang/Nim#16607 - when true or not defined(release): - initialized: bool - fd: T + shiftedFd: FDImpl ## The file descriptor mapped to a different range that + ## is invalid when zero-ed. + +func fd*[T](h: Handle[T]): T {.inline.} = + ## Returns the resource handle held by `h`. + ## + ## The returned handle will stay valid for the duration of `h`. + ## + ## **Note**: Do **not** close the returned handle. If ownership is wanted, + ## use `takeFd` instead. + getFdImpl() + +func `fd=`[T](h: var Handle[T], fd: T) {.inline.} = + ## Set the handle to be tracked. This does not destroy the previous handle. + setFdImpl() + +func takeFd*[T](h: var Handle[T]): T {.inline.} = + ## Returns the resource handle held by `h` and release ownership to the + ## caller. `h` will then be invalidated. + result = h.fd + `fd=`(h, InvalidFD.T) proc close*[T](h: var Handle[T]) {.inline.} = ## Close the handle owned by `h` and invalidating it. @@ -140,18 +157,10 @@ proc close*[T](h: var Handle[T]) {.inline.} = close h.fd finally: # Always invalidate `h.fd` to avoid double-close on destruction. - h.fd = InvalidFD.T + `fd=`(h, InvalidFD.T) proc `=destroy`[T](h: var Handle[T]) = ## Destroy the file handle. - when false: - # TODO: Once nim-lang/Nim#16607 is fixed, make this into a debug check - assert h.initialized, "Handle was not initialized before destruction, this is a compiler bug." - else: - # Walkaround for nim-lang/Nim#16607 - if not h.initialized: - return - if h.fd != InvalidFD: close h @@ -163,31 +172,11 @@ proc `=copy`*[T](dest: var Handle[T], src: Handle[T]) {.error.} proc initHandle*[T](fd: T): Handle[T] {.inline.} = ## Creates a `Handle` owning the passed `fd`. The `fd` shall then be freed ## automatically when the `Handle` go out of scope. - when false: - Handle[T](fd: fd) - else: - Handle[T](initialized: true, fd: fd) + `fd=`(result, fd) proc newHandle*[T](fd: T): ref Handle[T] {.inline.} = ## Creates a `ref Handle` owning the passed `fd`. The `fd` shall then be ## freed automatically when the returned `ref Handle[T]` is collected by the ## GC. - when false: - (ref Handle[T])(fd: fd) - else: - (ref Handle[T])(initialized: true, fd: fd) - -proc get*[T](h: Handle[T]): T {.inline.} = - ## Returns the resource handle held by `h`. - ## - ## The returned handle will stay valid for the duration of `h`. - ## - ## **Note**: Do **not** close the returned handle. If ownership is wanted, - ## use `take` instead. - h.fd - -proc take*[T](h: var Handle[T]): T {.inline.} = - ## Returns the resource handle held by `h` and release ownership to the - ## caller. `h` will then be invalidated. - result = h.fd - h.fd = InvalidFD.T + new result + result[] = initHandle(fd) diff --git a/src/sys/ioqueue.nim b/src/sys/ioqueue.nim index 8da33f3..ff3f49d 100644 --- a/src/sys/ioqueue.nim +++ b/src/sys/ioqueue.nim @@ -191,6 +191,6 @@ proc unregister*(fd: AnyFD) = ## *after* it and its duplicates are closed. unregisterImpl() -proc unregister*(fd: Handle[AnyFD]) = +proc unregister*(handle: Handle[AnyFD]) = ## An overload of `unregister` for `Handle` - ioqueue.unregister(fd.get) + ioqueue.unregister(handle.fd) diff --git a/src/sys/ioqueue/iocp.nim b/src/sys/ioqueue/iocp.nim index a6e55f7..5c06f5a 100644 --- a/src/sys/ioqueue/iocp.nim +++ b/src/sys/ioqueue/iocp.nim @@ -102,7 +102,7 @@ proc poll(runnable: var seq[Continuation], timeout = none(Duration)) {.used.} = # Obtain completion events var selected: ULONG if GetQueuedCompletionStatusEx( - wincore.Handle(eq.iocp.get), addr eq.eventBuffer[0], ULONG(eq.eventBuffer.len), + wincore.Handle(eq.iocp.fd), addr eq.eventBuffer[0], ULONG(eq.eventBuffer.len), addr selected, timeout, wincore.FALSE ) == wincore.FALSE: let errorCode = GetLastError() @@ -150,7 +150,7 @@ proc persist(fd: AnyFD) {.raises: [OSError].} = # Register the handle with IOCP if CreateIoCompletionPort( - wincore.Handle(fd), wincore.Handle(eq.iocp.get), ULongPtr(fd), 0 + wincore.Handle(fd), wincore.Handle(eq.iocp.fd), ULongPtr(fd), 0 ) == wincore.Handle(0): let errorCode = GetLastError() diff --git a/src/sys/private/files_posix.nim b/src/sys/private/files_posix.nim index 3475085..776d8a6 100644 --- a/src/sys/private/files_posix.nim +++ b/src/sys/private/files_posix.nim @@ -31,11 +31,11 @@ template newAsyncFileImpl() {.dirty.} = result = AsyncFile newFile(fd) template getFDImpl() {.dirty.} = - result = get f.handle + result = f.handle.fd template takeFDImpl() {.dirty.} = cleanupFile f - result = take f.handle + result = f.handle.takeFd proc commonRead(fd: FD, buf: pointer, len: Natural): int {.inline.} = ## A wrapper around posix.read() to retry on EINTR. diff --git a/src/sys/private/files_windows.nim b/src/sys/private/files_windows.nim index 65c1293..35c2839 100644 --- a/src/sys/private/files_windows.nim +++ b/src/sys/private/files_windows.nim @@ -46,11 +46,11 @@ template newAsyncFileImpl() {.dirty.} = result[] = AsyncFileImpl fileImpl template getFDImpl() {.dirty.} = - result = get f.handle + result = f.handle.fd template takeFDImpl() {.dirty.} = cleanupFile f - result = take f.handle + result = f.handle.takeFd proc initOverlapped(f: FileImpl, o: var Overlapped) {.inline.} = ## Initialize an overlapped object for I/O operations on `f` diff --git a/src/sys/private/handles_posix.nim b/src/sys/private/handles_posix.nim index e39b468..f986ad2 100644 --- a/src/sys/private/handles_posix.nim +++ b/src/sys/private/handles_posix.nim @@ -43,6 +43,17 @@ template setBlockingImpl() {.dirty.} = flags = flags or O_NONBLOCK posixChk fcntl(fd.cint, F_SETFL, flags), ErrorSetBlocking +template getFdImpl() {.dirty.} = + cast[T](cast[cuint](h.shiftedFd) - 1) + +template setFdImpl() {.dirty.} = + # The FD is casted to unsigned so that `high(cint)` can be mapped forward + # correctly. + # + # As all architectures running Linux uses two's complement, this also handles + # InvalidFD -> 0 correctly. + h.shiftedFd = cast[FDImpl](cast[cuint](fd) + 1) + when false: # NOTE: Staged until process spawning is added. template duplicateImpl() {.dirty.} = diff --git a/src/sys/private/handles_windows.nim b/src/sys/private/handles_windows.nim index 93c8639..e8e8f61 100644 --- a/src/sys/private/handles_windows.nim +++ b/src/sys/private/handles_windows.nim @@ -34,3 +34,9 @@ template setInheritableImpl() {.dirty.} = raise newOSError(GetLastError(), ErrorSetInheritable) else: {.error: "setInheritable is not available for this variant of FD".} + +template getFdImpl() {.dirty.} = + cast[T](cast[uint](h.shiftedFd - 1)) + +template setFdImpl() {.dirty.} = + h.shiftedFd = cast[FDImpl](cast[uint](fd) + 1) diff --git a/src/sys/private/ioqueue_bsd.nim b/src/sys/private/ioqueue_bsd.nim index cc724e0..c09b00d 100644 --- a/src/sys/private/ioqueue_bsd.nim +++ b/src/sys/private/ioqueue_bsd.nim @@ -90,7 +90,7 @@ proc queue(eq: var EventQueueImpl, cont: Continuation, fd: FD, event: ReadyEvent flags: EvAdd or EvEnable or EvDispatch ) - posixChk eq.kqueue.get.kevent(changeList = [kevent]), $Error.Queue + posixChk eq.kqueue.fd.kevent(changeList = [kevent]), $Error.Queue eq.waiters[fd] = Waiter(cont: cont, event: event) func toTimespec(d: Duration): Timespec = @@ -109,9 +109,9 @@ template pollImpl() {.dirty.} = # Obtain events from kevent let nevents = if timeout.isNone: - eq.kqueue.get.kevent(eventList = eq.eventBuffer) + eq.kqueue.fd.kevent(eventList = eq.eventBuffer) else: - eq.kqueue.get.kevent( + eq.kqueue.fd.kevent( eventList = eq.eventBuffer, timeout = toTimespec(timeout.get) ) @@ -155,7 +155,7 @@ template unregisterImpl() {.dirty.} = # If the FD is in the waiter list if fd in eq.waiters: # Deregister it from kqueue - let status = eq.kqueue.get.kevent([ + let status = eq.kqueue.fd.kevent([ # kqueue map events using a tuple of filter & identifier, so we # need to reproduce both to delete the event that we want. Kevent(ident: Ident(fd), filter: toFilter(eq.waiters[fd].event), diff --git a/src/sys/private/ioqueue_linux.nim b/src/sys/private/ioqueue_linux.nim index 35371de..c99b19b 100644 --- a/src/sys/private/ioqueue_linux.nim +++ b/src/sys/private/ioqueue_linux.nim @@ -79,7 +79,7 @@ func toEvents(ev: Ev): set[Event] = proc queue(eq: var EventQueueImpl, cont: Continuation, fd: AnyFD, event: ReadyEvent) = var epEvent = epoll.Event(events: toEv({event}), data: Data(fd: fd.cint)) # If adding `fd` to epoll fails - if eq.epoll.get.ctl(CtlAdd, fd, epEvent) == -1: + if eq.epoll.fd.ctl(CtlAdd, fd, epEvent) == -1: # In case `fd` was already registered if errno == EEXIST: # If there is a waiter in the queue @@ -88,7 +88,7 @@ proc queue(eq: var EventQueueImpl, cont: Continuation, fd: AnyFD, event: ReadyEv raise newException(ValueError, $Error.QueuedFD % $fd.cint) # Otherwise re-register our event - posixChk eq.epoll.get.ctl(CtlMod, fd, epEvent), $Error.Queue + posixChk eq.epoll.fd.ctl(CtlMod, fd, epEvent), $Error.Queue else: posixChk -1, $Error.Queue @@ -120,7 +120,7 @@ template pollImpl() {.dirty.} = eq.eventBuffer.setLen eq.waiters.len # Obtain the events that are ready - let selected = eq.epoll.get.wait(eq.eventBuffer, timeout) + let selected = eq.epoll.fd.wait(eq.eventBuffer, timeout) posixChk selected, $Error.Poll # Set the length of the buffer to the amount of events received eq.eventBuffer.setLen selected @@ -161,7 +161,7 @@ template unregisterImpl() {.dirty.} = # If the FD is in the waiter list if fd in eq.waiters: # Deregister it from epoll - let status = eq.epoll.get.ctl(CtlDel, fd, nil) + let status = eq.epoll.fd.ctl(CtlDel, fd, nil) if status == -1: # If the FD is in the waiter list but epoll said that it's never # registered or that it's invalid, then its already closed before being diff --git a/tests/handles/tdestroy.nim b/tests/handles/tdestroy.nim index fd81149..6da1344 100644 --- a/tests/handles/tdestroy.nim +++ b/tests/handles/tdestroy.nim @@ -33,25 +33,28 @@ suite "Test close() and Handle[T] destructions": expectDefect: close(SocketFD InvalidFD) - test "Handle[T] not destroyed before construction": - skip("nim-lang/Nim#16607") + test "Handle[T] initializes to invalid": + var handle: Handle[FD] - when not compileOption("assertions"): - skip("assertions are disabled") + check handle.fd == InvalidFD - proc pair(): tuple[a, b: Handle[FD]] = - result.a = initHandle(FD InvalidFD) - result.b = initHandle(FD InvalidFD) + test "Handle[T] can store the highest handle possible correctly": + const MaxFD = + when defined(posix): + FD(high cint) + elif defined(windows): + cast[FD](high uint) + else: + {.error: "A maximum FD is not defined for this platform".} - discard pair() + var handle = initHandle(MaxFD) - proc pairRef(): tuple[a, b: ref Handle[FD]] = - result.a = newHandle(FD InvalidFD) - result.b = newHandle(FD InvalidFD) - - discard pairRef() - when declared(GC_fullCollect): - GC_fullCollect() + try: + check handle.fd == MaxFD + finally: + # This handle is also invalid, so remove it to prevent Handle[T] from + # trying to close it. + discard handle.takeFd() var rd = FD InvalidFD @@ -92,4 +95,4 @@ suite "Test close() and Handle[T] destructions": expectDefect: close hrd - check hrd.get == InvalidFD + check hrd.fd == InvalidFD diff --git a/tests/ioqueue/asyncio.nim b/tests/ioqueue/asyncio.nim index 314c411..ef401a3 100644 --- a/tests/ioqueue/asyncio.nim +++ b/tests/ioqueue/asyncio.nim @@ -58,7 +58,7 @@ proc write*[T: byte or char](fd: Handle[FD], data: openArray[T]) {.sync.} = ## Write all bytes in `data` into `fd`. var totalWritten = 0 while totalWritten < data.len: - let written = write(fd.get.cint, data[totalWritten].unsafeAddr, data.len - totalWritten) + let written = write(fd.fd.cint, data[totalWritten].unsafeAddr, data.len - totalWritten) case written of -1: raise newIOError(totalWritten, errno.int32) @@ -69,7 +69,7 @@ proc read*[T: byte or char](fd: Handle[FD], buf: var openArray[T]): int {.sync.} ## Read all bytes from `fd` into `buf` until it's filled or there is ## nothing left to read while result < buf.len: - let readBytes = read(fd.get.cint, buf[result].addr, buf.len - result) + let readBytes = read(fd.fd.cint, buf[result].addr, buf.len - result) case readBytes of -1: raise newIOError(result, errno.int32) @@ -87,14 +87,14 @@ proc readAsync*(rd: ref Handle[FD], buf: ref string) {.asyncio.} = ## information outside of cps yet when defined(windows): let overlapped = new Overlapped - persist(rd.get) + persist(rd.fd) if ReadFile( - winim.Handle(rd.get), addr buf[0], DWORD(buf.len), nil, + winim.Handle(rd.fd), addr buf[0], DWORD(buf.len), nil, addr overlapped[] ) == winim.FALSE: let errorCode = GetLastError() if errorCode == ErrorIoPending: - wait(rd.get, overlapped) + wait(rd.fd, overlapped) else: discard "Error handling below" let errorCode = DWORD(overlapped.Internal) @@ -115,7 +115,7 @@ proc readAsync*(rd: ref Handle[FD], buf: ref string) {.asyncio.} = e.bytesTransferred += offset if e.errorCode == EAGAIN: offset = e.bytesTransferred - wait rd.get, Read + wait rd.fd, Read else: raise e @@ -123,14 +123,14 @@ proc writeAsync*(wr: ref Handle[FD], buf: string) {.asyncio.} = ## Write all bytes in `buf` into `wr` asynchronously when defined(windows): let overlapped = new Overlapped - persist(wr.get) + persist(wr.fd) if WriteFile( - winim.Handle(wr.get), unsafeAddr buf[0], DWORD(buf.len), nil, + winim.Handle(wr.fd), unsafeAddr buf[0], DWORD(buf.len), nil, addr overlapped[] ) == winim.FALSE: let errorCode = GetLastError() if errorCode == ErrorIoPending: - wait(wr.get, overlapped) + wait(wr.fd, overlapped) else: discard "Error handling below" let errorCode = DWORD(overlapped.Internal) @@ -148,6 +148,6 @@ proc writeAsync*(wr: ref Handle[FD], buf: string) {.asyncio.} = e.bytesTransferred += offset if e.errorCode == EAGAIN: offset = e.bytesTransferred - wait wr.get, Write + wait wr.fd, Write else: raise e