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

unix: don't abort loop initalization #1522

Open
wants to merge 1 commit into
base: v1.x
Choose a base branch
from

Conversation

LinuxJedi
Copy link

There are several conditions which could cause uv_loop_init() (and by
extension uv_loop_new()) to trigger an SIGABRT. This includes mutex
initialization failure or running out of FDs.

This patch removes the abort()s and returns error codes instead.
Cleaning up prior initialization if something has actually failed.

Fixes #1519

@txdv
Copy link
Contributor

txdv commented Oct 9, 2014

Can you make a separate commit/pull request for the unix/thread.c changes?

There are several conditions which could cause uv_loop_init() (and by
extension uv_loop_new()) to trigger an SIGABRT.  This includes mutex
initialization failure or running out of FDs.

This patch removes the abort()s and returns error codes instead.
Cleaning up prior initialization if something has actually failed.
@LinuxJedi
Copy link
Author

Changed this pull request to just the loop.c file. Will submit the other shortly

@saghul
Copy link
Contributor

saghul commented Oct 13, 2014

There is also a signal watcher which is started for monitoring child processes which we should also check for failure.

@LinuxJedi
Copy link
Author

almost makes you wish C had exceptions :) I'll take a look ASAP.

@saghul
Copy link
Contributor

saghul commented Oct 13, 2014

Also, despite many hating goto, it would help here: do all cleanup at the end of the function, using different labels.

@LinuxJedi
Copy link
Author

well, I was thinking initially of calling uv__loop_close() for cleanup, but that function assumes that init was good, Maybe a cleanup function instead of goto?

@saghul
Copy link
Contributor

saghul commented Oct 13, 2014

There is not much to cleanup, the key thing is what needs to be cleaned up, depending on where the failure occurs.

@saghul
Copy link
Contributor

saghul commented Nov 26, 2014

Ping?

@LinuxJedi
Copy link
Author

Sorry! Between conference talks and project deadlines I haven't had a lot of time to revisit this. I'll aim to take another look in the next week.

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.

3 participants