mitigating the problem with unreachable UNC paths by leaving at least one slot available for local operations #1115

Author: rejetto

src/stat.ts

@@ -0,0 +1,42 @@
+import { defineConfig } from './config'
+import { Stats } from 'node:fs'
+import { haveTimeout, pendingPromise } from './cross'
+import { stat } from 'fs/promises'
+
+const fileTimeout = defineConfig('file_timeout', 3, x => x * 1000)
+
+// since nodejs' UV_THREADPOOL_SIZE is limited, avoid using multiple slots for the same UNC host, and always leave one free for local operations
+const poolSize = Number(process.env.UV_THREADPOOL_SIZE || 4)
+const previous = new Map<string, Promise<Stats>>() // wrapped promises with haveTimeout
+const working = new Set<Promise<Stats>>() // plain stat's promise
+export async function statWithTimeout(path: string) {
+    const uncHost = /^\\\\([^\\]+)\\/.exec(path)?.[1]
+    if (!uncHost)
+        return haveTimeout(fileTimeout.compiled(), stat(path))
+    const busy = process.env.HFS_PARALLEL_UNC ? null : previous.get(uncHost) // by default we serialize requests on the same UNC host, to keep threadpool usage low
+    const ret = pendingPromise<Stats>()
+    previous.set(uncHost, ret) // reserve the slot before starting the operation
+    const err = await busy?.then(() => false, e => e.message === 'timeout' && e) // only timeout error is shared with pending requests
+    if (err) {
+        if (previous.get(uncHost) === ret) // but we don't want to block forever, only involve those that were already waiting
+            previous.delete(uncHost)
+        ret.reject(err)
+        return ret
+    }
+    while (working.size >= poolSize - 1) // always leave one slot free for local operations
+        await Promise.race(working.values()).catch(() => {}) // we are assuming UV_THREADPOOL_SIZE > 1, otherwise race() will deadlock
+    const op = stat(path)
+    working.add(op)
+    try {
+        ret.resolve(await haveTimeout(fileTimeout.compiled(),
+            op.finally(() => working.delete(op)) ))
+    }
+    catch (e) {
+        ret.reject(e)
+    }
+    finally {
+        if (previous.get(uncHost) === ret)
+            previous.delete(uncHost)
+    }
+    return ret
+}

src/util-files.ts

@@ -1,30 +1,18 @@
 // This file is part of HFS - Copyright 2021-2023, Massimo Melina <[email protected]> - License https://www.gnu.org/licenses/gpl-3.0.txt
 
-import { access, mkdir, readFile, stat } from 'fs/promises'
-import { Promisable, try_, wait, isWindowsDrive, haveTimeout, splitAt } from './cross'
-import { defineConfig } from './config'
+import { access, mkdir, readFile } from 'fs/promises'
+import { Promisable, try_, wait, isWindowsDrive } from './cross'
 import { createWriteStream, mkdirSync, watch, ftruncate } from 'fs'
 import { basename, dirname } from 'path'
 import glob from 'fast-glob'
 import { IS_WINDOWS } from './const'
 import { once } from 'events'
 import { Readable } from 'stream'
+import { statWithTimeout } from './stat'
 // @ts-ignore
 import unzipper from 'unzip-stream'
 
-const fileTimeout = defineConfig('file_timeout', 3, x => x * 1000)
-
-// since nodejs' UV_THREADPOOL_SIZE is 4 by default, avoid using multiple slots for the same UNC host. This doesn't solve the problem but at least mitigates it.
-const uncStatWaiting = new Map<string, Promise<any>>()
-export async function statWithTimeout(path: string) {
-    const uncHost = path.startsWith('\\\\') && splitAt('\\', path.slice(2))[0]
-    if (uncHost)
-        await uncStatWaiting.get(uncHost)
-    const op = stat(path)
-    if (uncHost)
-        uncStatWaiting.set(uncHost, op.finally(() => uncStatWaiting.delete(uncHost)).catch(() => {}))
-    return haveTimeout(fileTimeout.compiled(), op)
-}
+export { statWithTimeout }
 
 export async function isDirectory(path: string) {
     try { return (await statWithTimeout(path)).isDirectory() }
@@ -174,4 +162,4 @@ export async function parseFile<T>(path: string, parse: (path: string) => T) {
 
 export async function parseFileContent<T>(path: string, parse: (raw: Buffer) => T) {
     return parseFile(path, () => readFile(path).then(parse))
-}
+}

src/vfs.ts

@@ -172,7 +172,10 @@ export async function getNodeByName(name: string, parent: VfsNode) {
 }
 
 export let vfs: VfsNode = {}
-defineConfig('vfs', vfs).sub(reviewVfs)
+defineConfig('vfs', vfs).sub(async x => {
+    await reviewVfs(x)
+    console.log('VFS ready')
+})
 
 async function reviewVfs(data=vfs) {
     await (async function recur(node) {