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

Temporary fix to multiple initialisation of eio - pthread-specific #235

Closed
wants to merge 2 commits into from

Conversation

paddybyers
Copy link
Contributor

Issue summary is here:

paddybyers/node#15

* race conditions. See Node's test/simple/test-eio-race.js
*/
eio_set_max_poll_reqs(10);
pthread_once(&eio_initialised, eio_init_once);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pthread_once() is not fork-safe. If another thread calls fork() right when pthread_once() is doing its magic, eio_initialised will be in an undefined state.

It doesn't matter for Node because we always execve() immediately afterwards but it will be a problem in other applications that depend on libuv.

Then again, there is probably not much we can do about it. We can re-initialize libuv mutexes and guards in pthread_atfork() handlers but libeio uses mutexes internally that are out of our control.

Consider this the ramblings of a (not so) old man.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all platforms have pthread_atfork() either.

The other way to approach it is to pass the buck to the caller, just like eio does, and expose an initialiser that you say must only be called once.

@paddybyers
Copy link
Contributor Author

It's more complex than I thought and that second commit doesn't work. I commented here:
paddybyers/node#14 (comment)

The first commit is OK but the second one is not.

I will revert that .

@bnoordhuis
Copy link
Contributor

Closing, a newer version of this PR landed in abf9654.

@bnoordhuis bnoordhuis closed this Feb 26, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants