Skip to content

Add checks for PROXY_SYNC_ASYNC#26209

Open
stephenduong1004 wants to merge 1 commit intoemscripten-core:mainfrom
stephenduong1004:patch-5
Open

Add checks for PROXY_SYNC_ASYNC#26209
stephenduong1004 wants to merge 1 commit intoemscripten-core:mainfrom
stephenduong1004:patch-5

Conversation

@stephenduong1004
Copy link
Contributor

Check if the returned value is actually a Promise before attempting to use it as one.

I got en error when using fd_sync without ASYNCIFY: https://github.com/emscripten-core/emscripten/blob/main/src/lib/libwasi.js#L556. It is assigned __proxy: 'sync' (https://github.com/emscripten-core/emscripten/blob/main/src/lib/libcore.js#L2627), making it being proxied using PROXY_SYNC_ASYNC mode while not being a Promise.

Check if the returned value is actually a Promise before attempting to use it as one.

I got en error when using `fd_sync` without ASYNCIFY: https://github.com/emscripten-core/emscripten/blob/main/src/lib/libwasi.js#L556. It is assigned `__proxy: 'sync'` (https://github.com/emscripten-core/emscripten/blob/main/src/lib/libcore.js#L2627), making it being proxied using PROXY_SYNC_ASYNC mode while not being a Promise.
@sbc100
Copy link
Collaborator

sbc100 commented Feb 3, 2026

I think maybe we should instead mark the function as async (using the JS async keyword). That way it will always return a promise. See other places where we do {{{ asyncIf(ASYNCIFY) }}}.

Also, we should add a test for this. Can you explain how you were able to reproduce?

sbc100 added a commit to sbc100/emscripten that referenced this pull request Feb 3, 2026
This function is marked as `__async: 'auto'` which means it should
return a promise when run in proxied mode on the main thread.  However
it wasn't always returning a promise.   Adding the `async` keyword
is the simplest way to ensure it always returns a promise.

Also, update the `wrapSyscallFunction` function to preserve the async
keyword.

Replaces: emscripten-core#26209
sbc100 added a commit to sbc100/emscripten that referenced this pull request Feb 3, 2026
This function is marked as `__async: 'auto'` which means it should
return a promise when run in proxied mode on the main thread.  However
it wasn't always returning a promise.   Adding the `async` keyword
is the simplest way to ensure it always returns a promise.

Also, update the `wrapSyscallFunction` function to preserve the async
keyword.

Replaces: emscripten-core#26209
sbc100 added a commit to sbc100/emscripten that referenced this pull request Feb 3, 2026
This function is marked as `__async: 'auto'` which means it should
return a promise when run in proxied mode on the main thread.  However
it wasn't always returning a promise.   Adding the `async` keyword
is the simplest way to ensure it always returns a promise.

Also, update the `wrapSyscallFunction` function to preserve the async
keyword.

Replaces: emscripten-core#26209
sbc100 added a commit to sbc100/emscripten that referenced this pull request Feb 5, 2026
This function is marked as `__async: 'auto'` which means it should
return a promise when run in proxied mode on the main thread.  However
it wasn't always returning a promise.   Adding the `async` keyword
is the simplest way to ensure it always returns a promise.

Also, update the `wrapSyscallFunction` function to preserve the async
keyword.

Replaces: emscripten-core#26209
sbc100 added a commit that referenced this pull request Feb 5, 2026
This function is marked as `__async: 'auto'` which means it should
return a promise when run in proxied mode.

Also, update the `wrapSyscallFunction` function to preserve the async
keyword. This fix was needed when investigating using the `async`
keyword instead. Even though it didn't end up working in this case its
still a worthwhile fix.

Replaces: #26209

---------

Co-authored-by: Alon Zakai <alonzakai@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants