Type check Emscripten JS overrides and allow changes without php-wasm rebuilds

[brandonpayton]

Motivation for the change, related issues

Our system call overrides are not type checked, and any change to them requires rebuilding php-wasm JS files.

If we can separate our customization into TS modules and inject them into the php-wasm JS, we can benefit from type checking and reduce the number of slow php-wasm rebuilds we perform.

Implementation details

TBD

Testing Instructions (or ideally a Blueprint)

• CI

[brandonpayton]

At first, I thought we could make our php-wasm JS customizations (i.e., our fcntl() override) more testable by separating them into TS modules that can be examined independently and injected into php-wasm JS at run time.

But as I explore these changes, it looks like the most economical way to test our customizations is in their usual context with further integration tests.

That said, separating into TS modules would:

• Make critical php-wasm overrides subject to type checking
• Reduce php-wasm rebuilds. The main reason a rebuild would be required when our customizations change is if their bindings require additions (for example, relaying a new constant from Emscripten cDefs).

So I plan to continue working on this PR, but for the moment, I will pivot to:

• Add simple integration tests for file locking between multiple php-wasm instances.

[brandonpayton]

All tests are passing now. This PR needs some cleanup:

• Currently there are modules os-kernel-space.ts and os-user-space.ts. Is this reasonable terminology for folks?
  • "Kernel Space" describes elements of the platform that run with the full privileges of the OS. In this case, it describes things like the file manager that are owned at the top-level by Playground CLI.
  • "User Space" describes elements that run within a restricted environment within the OS. In this case, it describes the system call overrides that are owned and executed by individual workers, which are subservient to the top-level thread.
• The syscall overrides modules are currently under @php-wasm/universal and should be moved to @php-wasm/node.
• Update PR description with details.

Overall, I'm really happy with how this is turning out. I've had to fix a number of fcntl(), flock(), and fd_close() bugs while working on this PR, and most of the time, no php-wasm rebuilds were required for those fixes. It is much faster to iterate, and the type checking is a blessing as well. cc @adamziel @mho22

[brandonpayton]

Overall, I'm really happy with how this is turning out. I've had to fix a number of fcntl(), flock(), and fd_close() bugs while working on this PR, and most of the time, no php-wasm rebuilds were required for those fixes. It is much faster to iterate, and the type checking is a blessing as well. cc @adamziel @mho22

Right now, I'm focusing on these changes for file locking so we can implement proper native file locking in Windows, but after that, it would be amazing to convert all of our emscripten JS libraries to this model. Just adding type checking would probably reveal bugs, and avoiding more php-wasm rebuilds would be wonderful.

[mho22]

@brandonpayton I am curious how all of this works, you did a fantastic job here, but as someone who’s not very familiar with the file locking internals, I don’t fully understand everything that’s going on yet. Do you have a process for testing things out ? I’d love to better understand how it all works so I can help you with converting all the Emscripten JS libraries to this model in the future ? [ Anyways, I’ll follow this PR closely. ]

[adamziel]

oh this is fantastic

[adamziel]

nice, it will come handy for other syscalls. I've even drafted something mildly similar in #2988 – it will be cool to merge it into this API

[adamziel]

Should the JSPI code branch return here as a part of getting this PR ready for merging?

[brandonpayton]

@adamziel, what do you think of just using the synchronous remote API for both ASYNCIFY and JSPI branches in this case? Otherwise, the code we extracted from the syscall overrides needs to implement both sync/async paths which would be more complicated.

I think blocking on a synchronous API call will not be a problem once we move to using a single php-wasm instance per worker (which we could do as part of this PR).

[adamziel]

Oh, hm, that sounds pretty cool and somehow never occurred to me. I'm a bit worried about losing performance on rapid fcntl() calls if we used the sync API. If we can confirm the speed is similar in both scenarios, I'd be more than happy to drop two code paths in favor of one.

[brandonpayton]

OK, cool. 👏 One other good thing is that the sync calls will only be for the case where we fall back to FileLockManager. When each worker is using the native locking APIs directly, the remote API won't be used at all.

[adamziel]

Aa gotcha, then I think we're good. Thank you!

[adamziel]

Really nice direction @brandonpayton

[brandonpayton]

@brandonpayton I am curious how all of this works, you did a fantastic job here, but as someone who’s not very familiar with the file locking internals, I don’t fully understand everything that’s going on yet.

Some of this will be changing soon, so I'll describe the way it works today and where we are headed.

How it works today

We override the fcntl64() and flock() system calls provided by Emscripten. fcntl64() is overridden in a JS library, but flock() has to be overridden in php_wasm.c, which calls js_flock() from a JS library.

Our fcntl64() and flock()` overrides make calls to a remote FileLockManager API which is provided by the main Playground CLI thread. That way, the main thread can coordinate between lock attempts by different workers. The FileLockManager also attempts to lock the files natively to play more nicely with other native processes.

As part of this work, we also have to implement the behavior that locks are released when their associated file descriptor is closed, so we also override emscripten's fd_close() for this. We save the Emscripten's default implementation and call it in our override.

We also have to implement releasing locks when their owning process is terminated. For this, we created the JS library function js_release_file_locks() and call it from php_wasm.c when PHP is exiting.

Where we are headed

The current way native file locking works does not work in Windows. The reason is that Windows file locking is mandatory (enforced by the OS) where POSIX file locking is advisory (coordinated by the OS, not enforced, and apps control whether they respect locks). To implement native locking, today's FileLockManagerForNode opens its own file handle and attempts to natively lock it, but in Windows, the lock attempts are denied because, I believe, they conflict with the way the file was already opened by php-wasm. In our case, this is often the SQLite DB .ht.sqlite, but PHP does also attempt to obtain whole-file locks for other files in certain cases.

To solve this, we are moving to each php-wasm/node instance calling native locking APIs directly. This should be much simpler overall. We plan to keep FileLockManager as a backup for cases where the native locking APIs are not available.

Does this make sense? Is there something you would like to know more about?

Do you have a process for testing things out ?

Besides just running an unbuilt Playground CLI via npx nx unbuilt-asyncify playground-cli, we have some automated tests:

• packages/php-wasm/node/src/test/file-lock-manager-for-node.spec.ts
• packages/php-wasm/node/src/test/php-file-locking.spec.ts
• packages/playground/cli/tests/file-locking.spec.ts

I’d love to better understand how it all works so I can help you with converting all the Emscripten JS libraries to this model in the future ?

❤️ Thanks, @mho22!