Skip to content

Commit

Permalink
libobs/callback: Add signal reference counting
Browse files Browse the repository at this point in the history
Adds a simple signal reference counting function
(signal_handler_connect_ref) that makes it so that signals keep the
handler around until the all the signal itself is disconnected.  This
prevents potential crashes where a signal might try to disconnect after
a handler has already been destroyed (typically in C++ with
OBSSignalHandler helper objects, where destruction isn't guaranteed to
be predictable).

This also modifies OBSSignalHandler to use the reference-counting
connections.
  • Loading branch information
jp9000 committed Jun 3, 2018
1 parent 10fc206 commit 3a05cf6
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 21 deletions.
12 changes: 12 additions & 0 deletions docs/sphinx/reference-libobs-callback.rst
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,18 @@ Signals are used for all event-based callbacks.

---------------------

.. function:: void signal_handler_connect_ref(signal_handler_t *handler, const char *signal, signal_callback_t callback, void *data)

Connect a callback to a signal on a signal handler, and increments
the handler's internal reference counter, preventing it from being
destroyed until the signal has been disconnected.

:param handler: Signal handler object
:param callback: Signal callback
:param data: Private data passed the callback

---------------------

.. function:: void signal_handler_disconnect(signal_handler_t *handler, const char *signal, signal_callback_t callback, void *data)

Disconnects a callback from a signal on a signal handler.
Expand Down
80 changes: 61 additions & 19 deletions libobs/callback/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ struct signal_callback {
signal_callback_t callback;
void *data;
bool remove;
bool keep_ref;
};

struct signal_info {
Expand Down Expand Up @@ -96,6 +97,7 @@ struct global_callback_info {
struct signal_handler {
struct signal_info *first;
pthread_mutex_t mutex;
volatile long refs;

DARRAY(struct global_callback_info) global_callbacks;
pthread_mutex_t global_callbacks_mutex;
Expand Down Expand Up @@ -126,6 +128,7 @@ signal_handler_t *signal_handler_create(void)
{
struct signal_handler *handler = bzalloc(sizeof(struct signal_handler));
handler->first = NULL;
handler->refs = 1;

pthread_mutexattr_t attr;
if (pthread_mutexattr_init(&attr) != 0)
Expand All @@ -149,20 +152,25 @@ signal_handler_t *signal_handler_create(void)
return handler;
}

void signal_handler_destroy(signal_handler_t *handler)
static void signal_handler_actually_destroy(signal_handler_t *handler)
{
if (handler) {
struct signal_info *sig = handler->first;
while (sig != NULL) {
struct signal_info *next = sig->next;
signal_info_destroy(sig);
sig = next;
}
struct signal_info *sig = handler->first;
while (sig != NULL) {
struct signal_info *next = sig->next;
signal_info_destroy(sig);
sig = next;
}

da_free(handler->global_callbacks);
pthread_mutex_destroy(&handler->global_callbacks_mutex);
pthread_mutex_destroy(&handler->mutex);
bfree(handler);
da_free(handler->global_callbacks);
pthread_mutex_destroy(&handler->global_callbacks_mutex);
pthread_mutex_destroy(&handler->mutex);
bfree(handler);
}

void signal_handler_destroy(signal_handler_t *handler)
{
if (handler && os_atomic_dec_long(&handler->refs) == 0) {
signal_handler_actually_destroy(handler);
}
}

Expand Down Expand Up @@ -197,11 +205,12 @@ bool signal_handler_add(signal_handler_t *handler, const char *signal_decl)
return success;
}

void signal_handler_connect(signal_handler_t *handler, const char *signal,
signal_callback_t callback, void *data)
static void signal_handler_connect_internal(signal_handler_t *handler,
const char *signal, signal_callback_t callback, void *data,
bool keep_ref)
{
struct signal_info *sig, *last;
struct signal_callback cb_data = {callback, data, false};
struct signal_callback cb_data = {callback, data, false, keep_ref};
size_t idx;

if (!handler)
Expand All @@ -221,13 +230,28 @@ void signal_handler_connect(signal_handler_t *handler, const char *signal,

pthread_mutex_lock(&sig->mutex);

if (keep_ref)
os_atomic_inc_long(&handler->refs);

idx = signal_get_callback_idx(sig, callback, data);
if (idx == DARRAY_INVALID)
if (keep_ref || idx == DARRAY_INVALID)
da_push_back(sig->callbacks, &cb_data);

pthread_mutex_unlock(&sig->mutex);
}

void signal_handler_connect(signal_handler_t *handler, const char *signal,
signal_callback_t callback, void *data)
{
signal_handler_connect_internal(handler, signal, callback, data, false);
}

void signal_handler_connect_ref(signal_handler_t *handler, const char *signal,
signal_callback_t callback, void *data)
{
signal_handler_connect_internal(handler, signal, callback, data, true);
}

static inline struct signal_info *getsignal_locked(signal_handler_t *handler,
const char *name)
{
Expand All @@ -247,6 +271,7 @@ void signal_handler_disconnect(signal_handler_t *handler, const char *signal,
signal_callback_t callback, void *data)
{
struct signal_info *sig = getsignal_locked(handler, signal);
bool keep_ref = false;
size_t idx;

if (!sig)
Expand All @@ -256,13 +281,19 @@ void signal_handler_disconnect(signal_handler_t *handler, const char *signal,

idx = signal_get_callback_idx(sig, callback, data);
if (idx != DARRAY_INVALID) {
if (sig->signalling)
if (sig->signalling) {
sig->callbacks.array[idx].remove = true;
else
} else {
keep_ref = sig->callbacks.array[idx].keep_ref;
da_erase(sig->callbacks, idx);
}
}

pthread_mutex_unlock(&sig->mutex);

if (keep_ref && os_atomic_dec_long(&handler->refs) == 0) {
signal_handler_actually_destroy(handler);
}
}

static THREAD_LOCAL struct signal_callback *current_signal_cb = NULL;
Expand All @@ -280,6 +311,7 @@ void signal_handler_signal(signal_handler_t *handler, const char *signal,
calldata_t *params)
{
struct signal_info *sig = getsignal_locked(handler, signal);
long remove_refs = 0;

if (!sig)
return;
Expand All @@ -298,8 +330,12 @@ void signal_handler_signal(signal_handler_t *handler, const char *signal,

for (size_t i = sig->callbacks.num; i > 0; i--) {
struct signal_callback *cb = sig->callbacks.array+i-1;
if (cb->remove)
if (cb->remove) {
if (cb->keep_ref)
remove_refs++;

da_erase(sig->callbacks, i-1);
}
}

sig->signalling = false;
Expand Down Expand Up @@ -331,6 +367,12 @@ void signal_handler_signal(signal_handler_t *handler, const char *signal,
}

pthread_mutex_unlock(&handler->global_callbacks_mutex);

if (remove_refs) {
os_atomic_set_long(&handler->refs,
os_atomic_load_long(&handler->refs) -
remove_refs);
}
}

void signal_handler_connect_global(signal_handler_t *handler,
Expand Down
2 changes: 2 additions & 0 deletions libobs/callback/signal.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ static inline bool signal_handler_add_array(signal_handler_t *handler,

EXPORT void signal_handler_connect(signal_handler_t *handler,
const char *signal, signal_callback_t callback, void *data);
EXPORT void signal_handler_connect_ref(signal_handler_t *handler,
const char *signal, signal_callback_t callback, void *data);
EXPORT void signal_handler_disconnect(signal_handler_t *handler,
const char *signal, signal_callback_t callback, void *data);

Expand Down
4 changes: 2 additions & 2 deletions libobs/obs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class OBSSignal {
callback (callback_),
param (param_)
{
signal_handler_connect(handler, signal, callback, param);
signal_handler_connect_ref(handler, signal, callback, param);
}

inline void Disconnect()
Expand All @@ -236,7 +236,7 @@ class OBSSignal {
signal = signal_;
callback = callback_;
param = param_;
signal_handler_connect(handler, signal, callback, param);
signal_handler_connect_ref(handler, signal, callback, param);
}

OBSSignal(const OBSSignal&) = delete;
Expand Down

0 comments on commit 3a05cf6

Please sign in to comment.