diff --git a/LibOS/shim/include/shim_signal.h b/LibOS/shim/include/shim_signal.h index cab49c22..9e42af2c 100644 --- a/LibOS/shim/include/shim_signal.h +++ b/LibOS/shim/include/shim_signal.h @@ -98,12 +98,10 @@ struct shim_signal { PAL_CONTEXT * pal_context; }; -#define MAX_SIGNAL_LOG 32 - -struct shim_signal_log { - struct atomic_int head, tail; - struct shim_signal * logs[MAX_SIGNAL_LOG]; -}; +struct shim_signal_log; +struct shim_signal_log* signal_logs_alloc(void); +void signal_logs_free(struct shim_signal_log* signal_log); +bool signal_logs_pending(const struct shim_signal_log* signal_log, int sig); extern const char * const siglist[NUM_KNOWN_SIGS + 1]; diff --git a/LibOS/shim/include/shim_thread.h b/LibOS/shim/include/shim_thread.h index 0e2f4d37..1eb85b8f 100644 --- a/LibOS/shim/include/shim_thread.h +++ b/LibOS/shim/include/shim_thread.h @@ -192,9 +192,10 @@ void set_cur_thread (struct shim_thread * thread) thread->shim_tcb = tcb; tid = thread->tid; - if (!is_internal(thread) && !thread->signal_logs) - thread->signal_logs = malloc(sizeof(struct shim_signal_log) * - NUM_SIGS); + if (!is_internal(thread) && !thread->signal_logs) { + thread->signal_logs = signal_logs_alloc(); + assert(thread->signal_logs); /* FIXME on ENOMEM */ + } } else if (tcb->tp) { put_thread(tcb->tp); tcb->tp = NULL; diff --git a/LibOS/shim/src/bookkeep/shim_signal.c b/LibOS/shim/src/bookkeep/shim_signal.c index aec676dc..4b118763 100644 --- a/LibOS/shim/src/bookkeep/shim_signal.c +++ b/LibOS/shim/src/bookkeep/shim_signal.c @@ -42,6 +42,54 @@ typedef void (*__rt_sighandler_t)(int, siginfo_t*, void*); static __rt_sighandler_t default_sighandler[NUM_SIGS]; +#define MAX_SIGNAL_LOG 32 + +struct shim_signal_log { + /* FIXME: This whole structure needs a rewrite, it can't be implemented correctly lock-free. */ + /* + * ring buffer of pending same-type signals (e.g. all pending SIGINTs). + * [tail, head) for used area (with wrap around) + * [head, tail) for free area (with wrap around) + */ + struct atomic_int head; + struct atomic_int tail; + struct shim_signal* logs[MAX_SIGNAL_LOG]; +}; + +struct shim_signal_log* signal_logs_alloc(void) { + struct shim_signal_log* signal_logs = malloc(sizeof(*signal_logs) * NUM_SIGS); + if (!signal_logs) + return NULL; + for (int sig = 0; sig < NUM_SIGS; sig++) { + atomic_set(&signal_logs[sig].head, 0); + atomic_set(&signal_logs[sig].tail, 0); + } + return signal_logs; +} + +void signal_logs_free(struct shim_signal_log* signal_logs) { + for (int sig = 0; sig < NUM_SIGS; sig++) { + struct shim_signal_log* log = &signal_logs[sig]; + int tail = atomic_read(&log->tail); + int head = atomic_read(&log->head); + if (head < tail) { + for (int i = tail; i < MAX_SIGNAL_LOG; i++) { + free(log->logs[i]); + } + tail = 0; + } + for (int i = tail; i < head; i++) { + free(log->logs[i]); + } + } + free(signal_logs); +} + +bool signal_logs_pending(const struct shim_signal_log* signal_log, int sig) { + /* FIXME: race condition between reading two atomic variables */ + return atomic_read(&signal_log[sig - 1].tail) != atomic_read(&signal_log[sig - 1].head); +} + static struct shim_signal ** allocate_signal_log (struct shim_thread * thread, int sig) { @@ -49,24 +97,25 @@ allocate_signal_log (struct shim_thread * thread, int sig) return NULL; struct shim_signal_log * log = &thread->signal_logs[sig - 1]; - int head, tail, old_tail; + int tail, head, old_head; + /* FIXME: race condition between allocating the slot and populating the slot. */ do { - head = atomic_read(&log->head); - old_tail = tail = atomic_read(&log->tail); + tail = atomic_read(&log->tail); + old_head = head = atomic_read(&log->head); - if (head == tail + 1 || (!head && tail == (MAX_SIGNAL_LOG - 1))) + if (tail == head + 1 || (!tail && head == (MAX_SIGNAL_LOG - 1))) return NULL; - tail = (tail == MAX_SIGNAL_LOG - 1) ? 0 : tail + 1; - } while (atomic_cmpxchg(&log->tail, old_tail, tail) == tail); + head = (head == MAX_SIGNAL_LOG - 1) ? 0 : head + 1; + } while (atomic_cmpxchg(&log->head, old_head, head) == head); - debug("signal_logs[%d]: head=%d, tail=%d (counter = %ld)\n", sig - 1, - head, tail, thread->has_signal.counter + 1); + debug("signal_logs[%d]: tail=%d, head=%d (counter = %ld)\n", sig - 1, + tail, head, thread->has_signal.counter + 1); atomic_inc(&thread->has_signal); - return &log->logs[old_tail]; + return &log->logs[old_head]; } static struct shim_signal * @@ -74,28 +123,29 @@ fetch_signal_log (struct shim_thread * thread, int sig) { struct shim_signal_log * log = &thread->signal_logs[sig - 1]; struct shim_signal * signal = NULL; - int head, tail, old_head; + int tail, head, old_tail; + /* FIXME: race condition between finding the slot and clearing the slot. */ while (1) { - old_head = head = atomic_read(&log->head); - tail = atomic_read(&log->tail); + old_tail = tail = atomic_read(&log->tail); + head = atomic_read(&log->head); - if (head == tail) + if (tail == head) return NULL; - if (!(signal = log->logs[head])) + if (!(signal = log->logs[tail])) return NULL; - log->logs[head] = NULL; - head = (head == MAX_SIGNAL_LOG - 1) ? 0 : head + 1; + log->logs[tail] = NULL; + tail = (tail == MAX_SIGNAL_LOG - 1) ? 0 : tail + 1; - if (atomic_cmpxchg(&log->head, old_head, head) == old_head) + if (atomic_cmpxchg(&log->tail, old_tail, tail) == old_tail) break; - log->logs[old_head] = signal; + log->logs[old_tail] = signal; } - debug("signal_logs[%d]: head=%d, tail=%d\n", sig -1, head, tail); + debug("signal_logs[%d]: tail=%d, head=%d\n", sig -1, tail, head); atomic_dec(&thread->has_signal); diff --git a/LibOS/shim/src/bookkeep/shim_thread.c b/LibOS/shim/src/bookkeep/shim_thread.c index dd8d123d..9bc348fa 100644 --- a/LibOS/shim/src/bookkeep/shim_thread.c +++ b/LibOS/shim/src/bookkeep/shim_thread.c @@ -101,8 +101,7 @@ struct shim_thread* lookup_thread(IDTYPE tid) { return thread; } -IDTYPE get_pid (void) -{ +static IDTYPE get_pid(void) { IDTYPE idx; while (1) { @@ -168,8 +167,18 @@ struct shim_thread * get_new_thread (IDTYPE new_tid) } struct shim_thread * thread = alloc_new_thread(); - if (!thread) + if (!thread) { + release_pid(new_tid); return NULL; + } + + thread->signal_logs = signal_logs_alloc(); + if (!thread->signal_logs) { + free(thread); + release_pid(new_tid); + return NULL; + } + struct shim_thread * cur_thread = get_cur_thread(); thread->tid = new_tid; @@ -228,22 +237,18 @@ struct shim_thread * get_new_thread (IDTYPE new_tid) } } - thread->vmid = cur_process.vmid; - thread->signal_logs = malloc(sizeof(struct shim_signal_log) * - NUM_SIGS); - if (!thread->signal_logs) { - goto out_error; - } if (!create_lock(&thread->lock)) { goto out_error; } + + thread->vmid = cur_process.vmid; thread->scheduler_event = DkNotificationEventCreate(PAL_TRUE); thread->exit_event = DkNotificationEventCreate(PAL_FALSE); thread->child_exit_event = DkNotificationEventCreate(PAL_FALSE); return thread; out_error: - free(thread->signal_logs); + signal_logs_free(thread->signal_logs); if (thread->handle_map) { put_handle_map(thread->handle_map); } @@ -370,7 +375,7 @@ void put_thread (struct shim_thread * thread) destroy_lock(&thread->lock); } - free(thread->signal_logs); + signal_logs_free(thread->signal_logs); free(thread); } } @@ -846,8 +851,9 @@ BEGIN_RS_FUNC(running_thread) thread->set_child_tid = NULL; } - thread->signal_logs = malloc(sizeof(struct shim_signal_log) * - NUM_SIGS); + thread->signal_logs = signal_logs_alloc(); + if (!thread->signal_logs) + return -ENOMEM; if (cur_thread) { PAL_HANDLE handle = DkThreadCreate(resume_wrapper, thread); diff --git a/LibOS/shim/src/sys/shim_sigaction.c b/LibOS/shim/src/sys/shim_sigaction.c index 59468f2b..ee5b498a 100644 --- a/LibOS/shim/src/sys/shim_sigaction.c +++ b/LibOS/shim/src/sys/shim_sigaction.c @@ -202,8 +202,7 @@ int shim_do_sigsuspend(const __sigset_t* mask) { /* return immediately on some pending unblocked signal */ for (int sig = 1; sig <= NUM_SIGS; sig++) { - if (atomic_read(&cur->signal_logs[sig - 1].head) != - atomic_read(&cur->signal_logs[sig - 1].tail)) { + if (signal_logs_pending(cur->signal_logs, sig)) { /* at least one signal of type sig... */ if (!__sigismember(mask, sig)) { /* ...and this type is not blocked in supplied mask */ @@ -245,8 +244,7 @@ int shim_do_sigpending(__sigset_t* set, size_t sigsetsize) { return 0; for (int sig = 1; sig <= NUM_SIGS; sig++) { - if (atomic_read(&cur->signal_logs[sig - 1].head) != - atomic_read(&cur->signal_logs[sig - 1].tail)) + if (signal_logs_pending(cur->signal_logs, sig)) __sigaddset(set, sig); } diff --git a/LibOS/shim/src/sys/shim_sleep.c b/LibOS/shim/src/sys/shim_sleep.c index c2bef558..19697804 100644 --- a/LibOS/shim/src/sys/shim_sleep.c +++ b/LibOS/shim/src/sys/shim_sleep.c @@ -44,8 +44,7 @@ static bool signal_pending(void) { } for (int sig = 1; sig <= NUM_SIGS; sig++) { - if (atomic_read(&cur->signal_logs[sig - 1].head) != - atomic_read(&cur->signal_logs[sig - 1].tail)) { + if (signal_logs_pending(cur->signal_logs, sig)) { /* at least one signal of type sig... */ if (!__sigismember(&cur->signal_mask, sig)) { /* ...and this type is not blocked */