Skip to content

Commit bdf820e

Browse files
adamzieladamziel
authored
PHP Node: Only consider NODEFS to be a shared filesystem (#2300)
## Motivation for the change, related issues #2231 overrides `FS.hashAddNode` with `function hashAddNodeIfNotSharedFS(node)` where additional logic applies if `is_shared_fs_node(node)` is true. Only NODEFS nodes were supposed to be considered as coming from a shared fs. Unfortunately, the internal logic of `is_shared_fs_node()` also returned true for MEMFS nodes. This caused a FS error 44 for the following operation where `runtime2` attempts to create a directory in a `/wordpress` directory mounted from `runtime1`: ```ts import { loadNodeRuntime } from "@php-wasm/node"; import { getLoadedRuntime } from "@php-wasm/universal"; const opts = { emscriptenOptions: { ENV: { DOCROOT: '/wordpress' } } }; const runtime1 = getLoadedRuntime(await loadNodeRuntime('8.3', opts)); runtime1.FS.mkdir("/wordpress"); const runtime2 = getLoadedRuntime(await loadNodeRuntime('8.3', opts)); runtime2.FS.mkdir("/wordpress"); runtime2.FS.mount( runtime2.PROXYFS, { root: '/wordpress', fs: runtime1.FS }, '/wordpress' ); // This works: // runtime1.FS.mkdir("/wordpress/wp-content"); // This doesn't: runtime2.FS.mkdir("/wordpress/wp-content"); ``` Specifically, the FS error 44 was triggered inside `is_shared_fs_node()` when calling NODEFS operations on these non-NODEFS nodes. ## Implementation details Adds a check confirming the shared node comes from NODEFS. ## Testing Instructions (or ideally a Blueprint) * Confirm the reproduction above works without errors. * Once #2285 lands, we'll be able to add a unit test cc @brandonpayton
1 parent eb6d209 commit bdf820e

File tree

9 files changed

+156
-36
lines changed

9 files changed

+156
-36
lines changed

‎packages/php-wasm/compile/php/phpwasm-emscripten-library-file-locking-for-node.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,21 @@ const LibraryForFileLocking = {
4040
}
4141

4242
// Handle PROXYFS nodes which wrap other nodes.
43-
if (!node?.mount?.opts?.fs?.lookupPath) {
43+
if (!node?.mount?.opts?.fs?.lookupPath || !node?.mount?.type?.realPath) {
4444
return false;
4545
}
4646

47-
const vfsPath = NODEFS.realPath(node);
48-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
49-
return !!underlyingNode?.isSharedFS;
47+
// Only NODEFS can be shared between workers at the moment.
48+
if (node.mount.type !== NODEFS) {
49+
return false;
50+
}
51+
const vfsPath = node.mount.type.realPath(node);
52+
try {
53+
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
54+
return !!underlyingNode?.isSharedFS;
55+
} catch (e) {
56+
return false;
57+
}
5058
},
5159
is_path_to_shared_fs(path) {
5260
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_7_2.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5659,13 +5659,27 @@ export function init(RuntimeName, PHPLoader) {
56595659
if (node?.isSharedFS) {
56605660
return true;
56615661
}
5662+
56625663
// Handle PROXYFS nodes which wrap other nodes.
5663-
if (!node?.mount?.opts?.fs?.lookupPath) {
5664+
if (
5665+
!node?.mount?.opts?.fs?.lookupPath ||
5666+
!node?.mount?.type?.realPath
5667+
) {
5668+
return false;
5669+
}
5670+
5671+
// Only NODEFS can be shared between workers at the moment.
5672+
if (node.mount.type !== NODEFS) {
5673+
return false;
5674+
}
5675+
const vfsPath = node.mount.type.realPath(node);
5676+
try {
5677+
const underlyingNode =
5678+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5679+
return !!underlyingNode?.isSharedFS;
5680+
} catch (e) {
56645681
return false;
56655682
}
5666-
const vfsPath = NODEFS.realPath(node);
5667-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5668-
return !!underlyingNode?.isSharedFS;
56695683
},
56705684
is_path_to_shared_fs(path) {
56715685
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_7_3.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5659,13 +5659,27 @@ export function init(RuntimeName, PHPLoader) {
56595659
if (node?.isSharedFS) {
56605660
return true;
56615661
}
5662+
56625663
// Handle PROXYFS nodes which wrap other nodes.
5663-
if (!node?.mount?.opts?.fs?.lookupPath) {
5664+
if (
5665+
!node?.mount?.opts?.fs?.lookupPath ||
5666+
!node?.mount?.type?.realPath
5667+
) {
5668+
return false;
5669+
}
5670+
5671+
// Only NODEFS can be shared between workers at the moment.
5672+
if (node.mount.type !== NODEFS) {
5673+
return false;
5674+
}
5675+
const vfsPath = node.mount.type.realPath(node);
5676+
try {
5677+
const underlyingNode =
5678+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5679+
return !!underlyingNode?.isSharedFS;
5680+
} catch (e) {
56645681
return false;
56655682
}
5666-
const vfsPath = NODEFS.realPath(node);
5667-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5668-
return !!underlyingNode?.isSharedFS;
56695683
},
56705684
is_path_to_shared_fs(path) {
56715685
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_7_4.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5659,13 +5659,27 @@ export function init(RuntimeName, PHPLoader) {
56595659
if (node?.isSharedFS) {
56605660
return true;
56615661
}
5662+
56625663
// Handle PROXYFS nodes which wrap other nodes.
5663-
if (!node?.mount?.opts?.fs?.lookupPath) {
5664+
if (
5665+
!node?.mount?.opts?.fs?.lookupPath ||
5666+
!node?.mount?.type?.realPath
5667+
) {
5668+
return false;
5669+
}
5670+
5671+
// Only NODEFS can be shared between workers at the moment.
5672+
if (node.mount.type !== NODEFS) {
5673+
return false;
5674+
}
5675+
const vfsPath = node.mount.type.realPath(node);
5676+
try {
5677+
const underlyingNode =
5678+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5679+
return !!underlyingNode?.isSharedFS;
5680+
} catch (e) {
56645681
return false;
56655682
}
5666-
const vfsPath = NODEFS.realPath(node);
5667-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5668-
return !!underlyingNode?.isSharedFS;
56695683
},
56705684
is_path_to_shared_fs(path) {
56715685
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_8_0.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5677,13 +5677,27 @@ export function init(RuntimeName, PHPLoader) {
56775677
if (node?.isSharedFS) {
56785678
return true;
56795679
}
5680+
56805681
// Handle PROXYFS nodes which wrap other nodes.
5681-
if (!node?.mount?.opts?.fs?.lookupPath) {
5682+
if (
5683+
!node?.mount?.opts?.fs?.lookupPath ||
5684+
!node?.mount?.type?.realPath
5685+
) {
5686+
return false;
5687+
}
5688+
5689+
// Only NODEFS can be shared between workers at the moment.
5690+
if (node.mount.type !== NODEFS) {
5691+
return false;
5692+
}
5693+
const vfsPath = node.mount.type.realPath(node);
5694+
try {
5695+
const underlyingNode =
5696+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5697+
return !!underlyingNode?.isSharedFS;
5698+
} catch (e) {
56825699
return false;
56835700
}
5684-
const vfsPath = NODEFS.realPath(node);
5685-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5686-
return !!underlyingNode?.isSharedFS;
56875701
},
56885702
is_path_to_shared_fs(path) {
56895703
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_8_1.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5677,13 +5677,27 @@ export function init(RuntimeName, PHPLoader) {
56775677
if (node?.isSharedFS) {
56785678
return true;
56795679
}
5680+
56805681
// Handle PROXYFS nodes which wrap other nodes.
5681-
if (!node?.mount?.opts?.fs?.lookupPath) {
5682+
if (
5683+
!node?.mount?.opts?.fs?.lookupPath ||
5684+
!node?.mount?.type?.realPath
5685+
) {
5686+
return false;
5687+
}
5688+
5689+
// Only NODEFS can be shared between workers at the moment.
5690+
if (node.mount.type !== NODEFS) {
5691+
return false;
5692+
}
5693+
const vfsPath = node.mount.type.realPath(node);
5694+
try {
5695+
const underlyingNode =
5696+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5697+
return !!underlyingNode?.isSharedFS;
5698+
} catch (e) {
56825699
return false;
56835700
}
5684-
const vfsPath = NODEFS.realPath(node);
5685-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5686-
return !!underlyingNode?.isSharedFS;
56875701
},
56885702
is_path_to_shared_fs(path) {
56895703
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_8_2.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5677,13 +5677,27 @@ export function init(RuntimeName, PHPLoader) {
56775677
if (node?.isSharedFS) {
56785678
return true;
56795679
}
5680+
56805681
// Handle PROXYFS nodes which wrap other nodes.
5681-
if (!node?.mount?.opts?.fs?.lookupPath) {
5682+
if (
5683+
!node?.mount?.opts?.fs?.lookupPath ||
5684+
!node?.mount?.type?.realPath
5685+
) {
5686+
return false;
5687+
}
5688+
5689+
// Only NODEFS can be shared between workers at the moment.
5690+
if (node.mount.type !== NODEFS) {
5691+
return false;
5692+
}
5693+
const vfsPath = node.mount.type.realPath(node);
5694+
try {
5695+
const underlyingNode =
5696+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5697+
return !!underlyingNode?.isSharedFS;
5698+
} catch (e) {
56825699
return false;
56835700
}
5684-
const vfsPath = NODEFS.realPath(node);
5685-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5686-
return !!underlyingNode?.isSharedFS;
56875701
},
56885702
is_path_to_shared_fs(path) {
56895703
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_8_3.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5677,13 +5677,27 @@ export function init(RuntimeName, PHPLoader) {
56775677
if (node?.isSharedFS) {
56785678
return true;
56795679
}
5680+
56805681
// Handle PROXYFS nodes which wrap other nodes.
5681-
if (!node?.mount?.opts?.fs?.lookupPath) {
5682+
if (
5683+
!node?.mount?.opts?.fs?.lookupPath ||
5684+
!node?.mount?.type?.realPath
5685+
) {
5686+
return false;
5687+
}
5688+
5689+
// Only NODEFS can be shared between workers at the moment.
5690+
if (node.mount.type !== NODEFS) {
5691+
return false;
5692+
}
5693+
const vfsPath = node.mount.type.realPath(node);
5694+
try {
5695+
const underlyingNode =
5696+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5697+
return !!underlyingNode?.isSharedFS;
5698+
} catch (e) {
56825699
return false;
56835700
}
5684-
const vfsPath = NODEFS.realPath(node);
5685-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5686-
return !!underlyingNode?.isSharedFS;
56875701
},
56885702
is_path_to_shared_fs(path) {
56895703
const { node } = FS.lookupPath(path);

‎packages/php-wasm/node/jspi/php_8_4.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5677,13 +5677,27 @@ export function init(RuntimeName, PHPLoader) {
56775677
if (node?.isSharedFS) {
56785678
return true;
56795679
}
5680+
56805681
// Handle PROXYFS nodes which wrap other nodes.
5681-
if (!node?.mount?.opts?.fs?.lookupPath) {
5682+
if (
5683+
!node?.mount?.opts?.fs?.lookupPath ||
5684+
!node?.mount?.type?.realPath
5685+
) {
5686+
return false;
5687+
}
5688+
5689+
// Only NODEFS can be shared between workers at the moment.
5690+
if (node.mount.type !== NODEFS) {
5691+
return false;
5692+
}
5693+
const vfsPath = node.mount.type.realPath(node);
5694+
try {
5695+
const underlyingNode =
5696+
node.mount.opts.fs.lookupPath(vfsPath)?.node;
5697+
return !!underlyingNode?.isSharedFS;
5698+
} catch (e) {
56825699
return false;
56835700
}
5684-
const vfsPath = NODEFS.realPath(node);
5685-
const underlyingNode = node.mount.opts.fs.lookupPath(vfsPath)?.node;
5686-
return !!underlyingNode?.isSharedFS;
56875701
},
56885702
is_path_to_shared_fs(path) {
56895703
const { node } = FS.lookupPath(path);

0 commit comments

Comments
 (0)