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

Change fork() and system() to not return EAGAIN #7005

Open
wants to merge 1 commit into
base: incoming
from

Conversation

Projects
None yet
3 participants
@haukex

haukex commented Aug 16, 2018

The error code EAGAIN implies "try again later", but Emscripten's fork() will never succeed. Some C programs, when getting EAGAIN, will actually wait a few seconds and try again, sometimes many times. This patch changes fork() and system() to return ENOTSUP.

Any other error code is probably fine too, as long as it's not EAGAIN :-)

Change fork() and system() to not return EAGAIN
The error code EAGAIN implies "try again later", but Emscripten's fork() will never succeed. Some C programs, when getting EAGAIN, will actually wait a few seconds and try again, sometimes many times. This patch changes fork() and system() to return ENOTSUP.
@haukex

This comment has been minimized.

haukex commented Aug 17, 2018

I'll look at the test failures as soon as I can.

@kripken

This comment has been minimized.

Owner

kripken commented Aug 17, 2018

Thanks @haukex!

The change looks good. Test failures look like test outputs that need updating for the new error code. Should be easy, let me know if you run into a problem.

Also, please add yourself to AUTHORS.

@juj

This comment has been minimized.

Collaborator

juj commented Aug 19, 2018

This change will cause the behavior to no longer conform to the Open POSIX Test Suite. The rationale for passing EAGAIN on these, as well as pthread_create():

pthread_create: function(pthread_ptr, attr, start_routine, arg) {
if (typeof SharedArrayBuffer === 'undefined') {
err('Current environment does not support SharedArrayBuffer, pthreads are not available!');
return {{{ cDefine('EAGAIN') }}};
}

see also

97a091b#diff-be537b9f6dffd5af65894f62ef7416cfR27

comes from The Open Group's UNIX Specification at

http://pubs.opengroup.org/onlinepubs/7908799/xsh/fork.html
and http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_create.html

which has the wording

The pthread_create() function will fail if:
[EAGAIN]
The system lacked the necessary resources to create another thread, or the system-imposed limit on the total number of threads in a process PTHREAD_THREADS_MAX would be exceeded.

and

The fork() function will fail if:
[EAGAIN]
The system lacked the necessary resources to create another process, or the system-imposed limit on the total number of processes under execution system-wide or by a single user {CHILD_MAX} would be exceeded.

The OPEN Posix Test Suite enforces that no other error conditions are reported except those that are listed, which is why the intent is to output EAGAIN for "no more resources" to create threads.

I am open to diverging from this behavior, and returning ENOTSUP instead - could you do that for the pthread_join() entry point as well to be uniform?

See also occurrence of EAGAIN in tests, these look relevant:

/Users/clb/emsdk/emscripten/incoming/tests/pthread/test_pthread_malloc_free.cpp:
   45:       result = (rc != EAGAIN);

/Users/clb/emsdk/emscripten/incoming/tests/pthread/test_pthread_printf.cpp:
   27: 		assert(rc == EAGAIN);

/Users/clb/emsdk/emscripten/incoming/tests/pthread/test_pthread_supported.cpp:
   25: 	else assert(rc == EAGAIN);

/Users/clb/emsdk/emscripten/incoming/tests/pthread/test_pthread_thread_local_storage.cpp:
   54: 	else assert(rc == EAGAIN);

/Users/clb/emsdk/emscripten/incoming/tests/pthread/test_pthread_volatile.cpp:
   39:     int result = (rc != EAGAIN);

(change all of those to ENOTSUP)

@haukex

This comment has been minimized.

haukex commented Aug 22, 2018

Hi, sorry for the delayed response, it turns out my laptop isn't powerful enough to efficiently run the test suite, so I'll have to go to another machine. I'll update the branch with the changes (including pthread_join) as soon as I can, but it might be a few days until I can get around to it.

I agree that the change doesn't exactly conform to POSIX, but I figured that ENOMEM wasn't really a good option either.

By the way, thank you both for all your work, it's allowed me to port Perl to WebAssembly! https://webperl.zero-g.net/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment