tfdleak: fix flakyness on Windows (#14550)

* tfdleak_multiple: introduce stress tester for tfdleak

Imported from #14548 and tweaked for consumption by testament.

This test seems to be really good at bringing out the flakyness of
tfdleadk.

Co-authored-by: Timothee Cour <timothee.cour2@gmail.com>

* tfdleak: increase accuracy of the test on Windows

This commit implements a new testing strategy for Windows:
1. We duplicate the handle that will be tested and enable inheritance.
   This duplicate will serve as a reference handle.
2. In addition to checking whether the handle is valid, we also verify
   whether the handle is the same as the reference. This gives us
   complete certainty on whether the handle in question is inherited
   from the parent.
   A side effect is that this uses Windows 10+ APIs. But since
   this is just for the test, we don't have to be picky about it.

Ideally we would want to do something like this for other POSIX-based
system, but most of them lack a facility to do this, and as of writing
there isn't any false positive for them, so we won't need the additional
checks.

MemFile.fHandle will also no longer be tested, as this handle defaults
to being invalid.

Co-authored-by: Timothee Cour <timothee.cour2@gmail.com>

2 files changed, 64 insertions, 4 deletions

diff --git a/tests/stdlib/tfdleak.nim b/tests/stdlib/tfdleak.nim
index c4f144db5..5931be8c1 100644
--- a/tests/stdlib/tfdleak.nim
+++ b/tests/stdlib/tfdleak.nim
@@ -8,16 +8,41 @@ import os, osproc, strutils, nativesockets, net, selectors, memfiles,
        asyncdispatch, asyncnet
 when defined(windows):
   import winlean
+
+  # Note: Windows 10-only API
+  proc compareObjectHandles(first, second: Handle): WINBOOL
+                           {.stdcall, dynlib: "kernelbase",
+                             importc: "CompareObjectHandles".}
 else:
   import posix
 
 proc leakCheck(f: AsyncFD | int | FileHandle | SocketHandle, msg: string,
                expectLeak = defined(nimInheritHandles)) =
+  var args = @[$f.int, msg, $expectLeak]
+
+  when defined(windows):
+    var refFd: Handle
+    # NOTE: This function shouldn't be used to duplicate sockets,
+    #       as this function may mess with the socket internal refcounting.
+    #       but due to the lack of type segmentation in the stdlib for
+    #       Windows (AsyncFD can be a file or a socket), we will have to
+    #       settle with this.
+    #
+    #       Now, as a poor solution for the refcounting problem, we just
+    #       simply let the duplicated handle leak. This should not interfere
+    #       with the test since new handles can't occupy the slot held by
+    #       the leaked ones.
+    if duplicateHandle(getCurrentProcess(), f.Handle,
+                       getCurrentProcess(), addr refFd,
+                       0, 1, DUPLICATE_SAME_ACCESS) == 0:
+      raiseOSError osLastError(), "Couldn't create the reference handle"
+    args.add $refFd
+
   discard startProcess(
     getAppFilename(),
-    args = @[$f.int, msg, $expectLeak],
+    args = args,
     options = {poParentStreams}
-  ).waitForExit -1
+  ).waitForExit
 
 proc isValidHandle(f: int): bool =
   ## Check if a handle is valid. Requires OS-native handles.
@@ -72,7 +97,6 @@ proc main() =
     var mf = memfiles.open("__test_fdleak3", fmReadWrite, newFileSize = 1)
     defer: close mf
     when defined(windows):
-      leakCheck(mf.fHandle, "memfiles.open().fHandle", false)
       leakCheck(mf.mapHandle, "memfiles.open().mapHandle", false)
     else:
       leakCheck(mf.handle, "memfiles.open().handle", false)
@@ -105,7 +129,20 @@ proc main() =
       fd = parseInt(paramStr 1)
       expectLeak = parseBool(paramStr 3)
       msg = (if expectLeak: "not " else: "") & "leaked " & paramStr 2
-    if expectLeak xor fd.isValidHandle:
+    let validHandle =
+      when defined(windows):
+        # On Windows, due to the use of winlean, causes the program to open
+        # a handle to the various dlls that's loaded. This handle might
+        # collide with the handle sent for testing.
+        #
+        # As a walkaround, we pass an another handle that's purposefully leaked
+        # as a reference so that we can verify whether the "leaked" handle
+        # is the right one.
+        let refFd = parseInt(paramStr 4)
+        fd.isValidHandle and compareObjectHandles(fd, refFd) != 0
+      else:
+        fd.isValidHandle
+    if expectLeak xor validHandle:
       echo msg
 
 when isMainModule: main()
diff --git a/tests/stdlib/tfdleak_multiple.nim b/tests/stdlib/tfdleak_multiple.nim
new file mode 100644
index 000000000..51d291284
--- /dev/null
+++ b/tests/stdlib/tfdleak_multiple.nim
@@ -0,0 +1,23 @@
+import os, osproc, strutils
+
+const Iterations = 200
+
+proc testFdLeak() =
+  var count = 0
+  let
+    test = getAppDir() / "tfdleak"
+    exe = test.addFileExt(ExeExt).quoteShell
+    options = ["", "-d:nimInheritHandles"]
+  for opt in options:
+    let
+      run = "nim c $1 $2" % [opt, quoteShell test]
+      (output, status) = execCmdEx run
+    doAssert status == 0, "Test complination failed:\n$1\n$2" % [run, output]
+    for i in 1..Iterations:
+      let (output, status) = execCmdEx exe
+      doAssert status == 0, "Execution of " & exe & " failed"
+      if "leaked" in output:
+        count.inc
+    doAssert count == 0, "Leaked " & $count & " times"
+
+when isMainModule: testFdLeak()