Skip to content

PHP Node: Only consider NODEFS to be a shared filesystem #2300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

adamziel
Copy link
Collaborator

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:

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)

cc @brandonpayton

@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Package][@php-wasm] Node labels Jun 24, 2025
@adamziel adamziel merged commit bdf820e into trunk Jun 25, 2025
25 checks passed
@adamziel adamziel deleted the only-consider-nodefs-to-be-shared branch June 25, 2025 08:25
@adamziel
Copy link
Collaborator Author

I'll go ahead and merge to unblock #2281. We can always revert if something's off here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package][@php-wasm] Node [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant