-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Segmentation fault: 11 on OSX #4091
Comments
I was able to reproduce. Looks to be an issue with fsevents?
|
It's an issue with fsevents. How to reproduce? @evanlucas |
/cc @indutny - he wrote most of libuv's fsevents code. |
I will take a look at this, but I need a test case. @evanlucas could you please share it with me? |
@indutny you need OSX, and then just follow steps indicated in description. It's important to note that it doesn't happen on every run (let's say 1 per 3 runs), so if it's not exposed by first test run, try again, you'll definitely get it. |
The problem happens in |
Will look more into it. |
Strictly speaking
|
@medikoo may I ask you to give a try to the following patch? diff --git a/deps/uv/src/unix/fsevents.c b/deps/uv/src/unix/fsevents.c
index 8143f7c..1259754 100644
--- a/deps/uv/src/unix/fsevents.c
+++ b/deps/uv/src/unix/fsevents.c
@@ -76,6 +76,7 @@ typedef struct uv__cf_loop_state_s uv__cf_loop_state_t;
struct uv__cf_loop_signal_s {
QUEUE member;
uv_fs_event_t* handle;
+ unsigned int closing:1;
};
struct uv__fsevents_event_s {
@@ -98,7 +99,9 @@ struct uv__cf_loop_state_s {
/* Forward declarations */
static void uv__cf_loop_cb(void* arg);
static void* uv__cf_loop_runner(void* arg);
-static int uv__cf_loop_signal(uv_loop_t* loop, uv_fs_event_t* handle);
+static int uv__cf_loop_signal(uv_loop_t* loop,
+ uv_fs_event_t* handle,
+ unsigned int closing);
/* Lazy-loaded by uv__fsevents_global_init(). */
static CFArrayRef (*pCFArrayCreate)(CFAllocatorRef,
@@ -387,7 +390,8 @@ static void uv__fsevents_destroy_stream(uv_loop_t* loop) {
/* Runs in CF thread, when there're new fsevent handles to add to stream */
-static void uv__fsevents_reschedule(uv_fs_event_t* handle) {
+static void uv__fsevents_reschedule(uv_fs_event_t* handle,
+ unsigned int closing) {
uv__cf_loop_state_t* state;
QUEUE* q;
uv_fs_event_t* curr;
@@ -486,7 +490,7 @@ final:
*
* NOTE: This is coupled with `uv_sem_wait()` in `uv__fsevents_close`
*/
- if (!uv__is_active(handle))
+ if (closing)
uv_sem_post(&state->fsevent_sem);
}
@@ -676,7 +680,7 @@ void uv__fsevents_loop_delete(uv_loop_t* loop) {
if (loop->cf_state == NULL)
return;
- if (uv__cf_loop_signal(loop, NULL) != 0)
+ if (uv__cf_loop_signal(loop, NULL, 0) != 0)
abort();
uv_thread_join(&loop->cf_thread);
@@ -753,7 +757,7 @@ static void uv__cf_loop_cb(void* arg) {
if (s->handle == NULL)
pCFRunLoopStop(state->loop);
else
- uv__fsevents_reschedule(s->handle);
+ uv__fsevents_reschedule(s->handle, s->closing);
QUEUE_REMOVE(item);
uv__free(s);
@@ -762,7 +766,9 @@ static void uv__cf_loop_cb(void* arg) {
/* Runs in UV loop to notify CF thread */
-int uv__cf_loop_signal(uv_loop_t* loop, uv_fs_event_t* handle) {
+int uv__cf_loop_signal(uv_loop_t* loop,
+ uv_fs_event_t* handle,
+ unsigned int closing) {
uv__cf_loop_signal_t* item;
uv__cf_loop_state_t* state;
@@ -771,6 +777,7 @@ int uv__cf_loop_signal(uv_loop_t* loop, uv_fs_event_t* handle) {
return -ENOMEM;
item->handle = handle;
+ item->closing = closing;
uv_mutex_lock(&loop->cf_mutex);
QUEUE_INSERT_TAIL(&loop->cf_signals, &item->member);
@@ -833,7 +840,7 @@ int uv__fsevents_init(uv_fs_event_t* handle) {
/* Reschedule FSEventStream */
assert(handle != NULL);
- err = uv__cf_loop_signal(handle->loop, handle);
+ err = uv__cf_loop_signal(handle->loop, handle, 0);
if (err)
goto fail_loop_signal;
@@ -873,7 +880,7 @@ int uv__fsevents_close(uv_fs_event_t* handle) {
/* Reschedule FSEventStream */
assert(handle != NULL);
- err = uv__cf_loop_signal(handle->loop, handle);
+ err = uv__cf_loop_signal(handle->loop, handle, 1);
if (err)
return -err;
|
Appears to be working fine for me so far... |
Got to say, that if it will fix this - you have hit a pretty unexpected edge case! :) Good job. |
@indutny works great, I gave it a dozen of runs with even more tests involved, and no crash was observed. Great thanks! Will this fix also be ported to LTS 4.2? |
@medikoo definitely. Though, I will need to create a test case for libuv first ;) |
When `uv_fsevents_t` handle is closed immediately after initializing, there is a possibility that the `CFRunLoop`'s thread will process both of these events at the same time. `uv__is_active(handle)` will return `0`, and the `uv_close()` semaphore will be unblocked, leading to the use after free in node.js. See: nodejs/node#4091
Looks like the test is already in libuv, filed a PR: libuv/libuv#637 |
👍 |
When `uv_fsevents_t` handle is closed immediately after initializing, there is a possibility that the `CFRunLoop`'s thread will process both of these events at the same time. `uv__is_active(handle)` will return `0`, and the `uv_close()` semaphore will be unblocked, leading to the use after free in node.js. See: nodejs/node#4091 PR-URL: #637 Reviewed-By: Ben Noordhuis <[email protected]>
I think I have this one too. I am wrapping my node process with pm2 so it will just respawn. Is there instructions to recompile somewhere so I can verify? |
@bo01ean definitely! The fix has been already landed in master, so if you could just:
|
Have we updated libuv already? cc @evanlucas do we need to backport this to LTS? |
I'm not sure that LTS has had it back ported. Will check tomorrow and see. |
It looks like Argon is still using [email protected] (4c59407) so I would think it would need backporting. |
This should have been fixed in v4.2.5 and v5.3.0. Thanks! |
In one of the projects I have test which involves many watchers, and some race condition occurs.
Occasionally (let's say once per three times) test crashes with
Segmentation fault: 11
There's purely JS modules involved, and it's observable on OSX 10.11.1 and Node.js v5.1.0.
I was not able to produce any small test case for that. Best way to reproduce is to do:
(After test fails, to rerun it it's good to proceed with
rm -rf test && git checkout test
as broken test leaves temporary files).The text was updated successfully, but these errors were encountered: