Skip to content

Add support for setting socket's blocking mode through ioctl with FIO… - #24641

Merged
kripken merged 1 commit into
emscripten-core:mainfrom
inolen:inolen/ioctl_fionbio
Jul 2, 2025
Merged

Add support for setting socket's blocking mode through ioctl with FIO…#24641
kripken merged 1 commit into
emscripten-core:mainfrom
inolen:inolen/ioctl_fionbio

Conversation

@inolen

@inolen inolen commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator

…NBIO

Today I was recompiling quakejs (12 years later, better late than never :)) against the latest emscripten. Everything was smooth sailing, but I hit one issue because ioctl() used to be under-implemented, and now it's more fleshed out but missing support for FIONBIO (https://github.com/torvalds/linux/blob/66701750d5565c574af42bef0b789ce0203e3071/fs/ioctl.c#L341).

I don't have the tests working locally right now, so I'm crossing my fingers that some CI job kicks in here to help verify. I modified test_sockets_echo_server which had support for both Winsock / POSIX sockets, and shimmed it to use ioctl() on POSIX to help test this.

Edit: Of course, this doesn't actually change or implement any support for O_NONBLOCK inside of libsockfs, it's just enabling the flag to be set just as it would be through fcntl.

@inolen

inolen commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator Author

Any advice on these test failures? I spliced in the changes to add FIONBIO into struct_info.json, because gen_struct_info.py was throwing an error for me. Not sure if I missed something, or if they're unrelated.

@kripken

kripken commented Jul 1, 2025

Copy link
Copy Markdown
Member

(12 years later, better late than never :))

Hi @inolen ! 😄 Been a while, indeed...

Those test errors may be unrelated. Updating to latest main might fix them, I'm pressing the "Update branch" button now. If they don't go away, I'll take a closer look.

@inolen
inolen force-pushed the inolen/ioctl_fionbio branch from 79c0a5a to 22026c0 Compare July 1, 2025 23:15
@kripken
kripken merged commit 355f97b into emscripten-core:main Jul 2, 2025
30 checks passed
@kripken

kripken commented Jul 2, 2025

Copy link
Copy Markdown
Member

How does quakejs run now, btw?

@inolen

inolen commented Jul 3, 2025

Copy link
Copy Markdown
Collaborator Author

How does quakejs run now, btw?

Seemingly works fine, do you have anything in particular you're curious about?

@kripken

kripken commented Jul 7, 2025

Copy link
Copy Markdown
Member

I guess I was just wondering if performance was any different? (hopefully better...)

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