Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Attempt to make libuv fork safe. #1136

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yacobucci
Copy link

If threadpool is initialized try and make libuv fork safe. As a library
it's hard to predict what an application may do and a programmer not
realizing libuv may spawn threads may fork without an exec call. If this
occurs and threadpool is initialized the child will abort in shutdown as
the threads no longer exist and pthread_join fails.

This checkin doesn't work at the moment - the threadpool is now fork
safe, but on BSD kqueue is not and any loop will have an invalid kqueue

More info here: https://groups.google.com/forum/#!topic/libuv/jJcVpdflSIU
in the child.

If threadpool is initialized try and make libuv fork safe. As a library
it's hard to predict what an application may do and a programmer not
realizing libuv may spawn threads may fork without an exec call. If this
occurs and threadpool is initialized the child will abort in shutdown as
the threads no longer exist and pthread_join fails.

This checkin doesn't work at the moment - the threadpool is now fork
safe, but on BSD kqueue is not and any loop will have an invalid kqueue
in the child.
@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit yacobucci/libuv@7ffe3a0 has the following error(s):

  • Commit message must indicate the subsystem this commit changes

The following commiters were not found in the CLA:

  • Matthew Yacobucci

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@txdv
Copy link
Contributor

txdv commented Jul 7, 2014

General whitespace issue, you should go through the code and check for whitespaces.

Every worker is now going through a lock and unlock. I don't know how much of a performance drain that would is. Can you create a libuv test where you do a simple while loop of N times where in one you do nothing and in the other a lock and unlock subsequently?

In general I wonder why we would want to make libuv fork safe. As you said in the forum kqueue is not forkable, I doubt the others are and even if they are, you don't want to

@saghul
Copy link
Contributor

saghul commented Jul 7, 2014

@txdv this cannot land in its current form, it's Unix specific and the threadpool is shared now.

@txdv
Copy link
Contributor

txdv commented Jul 7, 2014

I wonder why he wants it all.

He said fork-exec is the only reason why you would want to fork and IMO that have basically the the same functionality as uv_spawn.

Could this performance wise be faster?

@brimworks
Copy link

FWIW, I ran into this when trying to write a program that "daemonize's" (fork, setsid, fork):

http://www.microhowto.info/howto/cause_a_process_to_become_a_daemon_in_c.html

...I know this is out-of-date advice since most Linux flavors use upstart, supervisord, or other process managers, but I was trying to write my daemon in a way that would work with SysV.

Thanks,
-Brian

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants