René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"`fs.glob`: return in cache-seen check aborts sibling processing, makes `test-fs-glob.mjs` flaky","articleBody":"### Version\n\nReproduced on Node.js v25.x (current main), present since the glob implementation was added.\n\n### Platform\n\n```text\nLinux (ext4/tmpfs) -- most reproducible there due to non-deterministic readdir ordering, but the bug exists on all platforms.\n```\n\n### Subsystem\n\nfs\n\n### What steps will reproduce the bug?\n\nThe bug is in lib/internal/fs/glob.js, in both #addSubpatterns and #iterateSubpatterns. The cache.seen check inside the children iteration loop uses return, which exits the entire method instead of skipping just the current child:\n\n```js\nfor (let i = 0; i \u003c children.length; i++) {\n  // ...\n  for (const index of pattern.indexes) {\n    if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) {\n      return; // \u003c-- exits the entire method, skipping ALL remaining children\n    }\n```\n\nThis can be triggered with **/../* patterns. Minimal reproduction:\n\n```js\nconst { mkdirSync, mkdtempSync, writeFileSync, globSync, rmSync } = require('fs');\nconst { tmpdir } = require('os');\nconst { join } = require('path');\n\nconst tmp = mkdtempSync(join(tmpdir(), 'glob-bug-'));\n\n// a/b/c/d  (nested dirs)\n// a/c/d/c/ (child \"c\" of a/c/d triggers \"..\" back to a/c, creating a duplicate queue entry)\n// a/x      (file)\n// a/z      (file)\nconst a = join(tmp, 'a');\nmkdirSync(join(a, 'b', 'c', 'd'), { recursive: true });\nmkdirSync(join(a, 'c', 'd', 'c'), { recursive: true });\nwriteFileSync(join(a, 'x'), '');\nwriteFileSync(join(a, 'z'), '');\n\n// Run many times -- result varies depending on readdir ordering\nfor (let i = 0; i \u003c 100; i++) {\n  const results = globSync('a/**/../*', { cwd: tmp }).sort();\n  for (const expected of ['a/b', 'a/c', 'a/x', 'a/z']) {\n    if (!results.includes(expected)) {\n      console.error(`Iteration ${i}: missing \"${expected}\" from results:`, results);\n      process.exit(1);\n    }\n  }\n}\nconsole.log('ok');\n\nrmSync(tmp, { recursive: true, force: true });\n```\nOn Linux with ext4/tmpfs, this frequently fails with a/c or other entries missing. On macOS (APFS), readdir ordering is more stable so it may require more iterations or a different directory structure.\n\n### How often does it reproduce? Is there a required condition?\n\nDepends on readdir ordering, which varies by filesystem. On Linux CI (ext4/tmpfs) it reproduces frequently. The required condition is a directory tree deep enough that the .. handler in the GLOBSTAR branch queues a path that was already queued by a parent directory's processing.\n\n### What is the expected behavior? Why is that the expected behavior?\n\n`globSync('a/**/../*', { cwd })` should return all entries reachable via `**/../*` regardless of readdir ordering. The `cache.seen` check should skip only the already-processed child, not abort processing of all remaining siblings.\n\n### What do you see instead?\n\nDepending on readdir ordering, some entries in the parent directory are missing from the results. The `test-fs-glob.mjs` test case for `a/!(symlink)/**/../*` is affected -- a/c and a/symlink can be missing.\n\n### Additional information\n\nI discovered this in Deno, due to flakiness when running `test-fs-glob.mjs`.\n\n### Root cause\n\nWhen a pattern contains `**/../*`:\n\n1. Processing directory `a/c` with GLOBSTAR queues three items via the `..` handler and #addSubpattern: `a/c` with `{*}`, `a` with `{*}`, and `a/c/d` with `{**}`\n2. The queue is LIFO (ArrayPrototypePop), so `a/c/d` is popped first. Its child `c` (directory) triggers the `..` handler again, which adds `a/c` with `{*}` to subpatterns a second time\n3. This second `a/c {*} ` gets pushed onto the queue after a `{*}`, so LIFO pops it before a `{*}`\n4. `a/c {*} ` is processed, adding `*` to `cache[\"a/c\"]`\n5. When a `{*}` is finally processed and iterates children of `a/`, child `c` has `entryPath = \"a/c\"` which is now in the cache. `cache.seen` returns true and return exits the method, skipping all remaining siblings (e.g., `a/x`, `a/z`, `a/symlink`)\n\nWhich siblings are skipped depends on readdir ordering, hence the flakiness.\n\n### Suggested fix\n\nRemove the cache.seen check from the children iteration loop entirely:\n\n```diff\n-      for (const index of pattern.indexes) {\n-        if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) {\n-          return;\n-        }\n+      for (const index of pattern.indexes) {\n```\n\nThe cache.add call at the top of #addSubpatterns/#iterateSubpatterns already prevents reprocessing and infinite recursion.\nThe children-level check is a redundant optimization that incorrectly prevents entries from being added to results when they\nwere previously traversed from a different context (e.g., a/b was recursed into via its own queue entry, but still needs to\nappear as a result when the parent a matches * against its children).\n\nThis needs to be applied in both #addSubpatterns (sync) and #iterateSubpatterns (async).\n\nFix PR in Deno: https://github.com/denoland/deno/issues/33372\n\nI used AI to generate this report and the fix in Deno.","author":{"url":"https://github.com/bartlomieju","@type":"Person","name":"bartlomieju"},"datePublished":"2026-04-22T19:50:37Z","url":"https://github.com/nodejs/node/issues/62897"}