Skip to content

Commit

Permalink
thread api abstraction. fix deadlock in debug code
Browse files Browse the repository at this point in the history
  • Loading branch information
@nickkelsey
nickkelsey committed Jun 12, 2017
1 parent e1076a2 commit f736fab
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 98 deletions.
83 changes: 42 additions & 41 deletions hdhomerun_debug.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ struct hdhomerun_debug_message_t

struct hdhomerun_debug_t
{
pthread_t thread;
thread_task_t thread;
volatile bool enabled;
volatile bool terminate;
char *prefix;

pthread_mutex_t print_lock;
pthread_mutex_t queue_lock;
pthread_mutex_t send_lock;
thread_mutex_t print_lock;
thread_mutex_t queue_lock;
thread_mutex_t send_lock;

thread_cond_t queue_cond;
struct hdhomerun_debug_message_t *queue_head;
Expand All @@ -68,7 +68,7 @@ struct hdhomerun_debug_t
struct hdhomerun_sock_t *sock;
};

static THREAD_FUNC_PREFIX hdhomerun_debug_thread_execute(void *arg);
static void hdhomerun_debug_thread_execute(void *arg);

struct hdhomerun_debug_t *hdhomerun_debug_create(void)
{
Expand All @@ -77,12 +77,12 @@ struct hdhomerun_debug_t *hdhomerun_debug_create(void)
return NULL;
}

pthread_mutex_init(&dbg->print_lock, NULL);
pthread_mutex_init(&dbg->queue_lock, NULL);
pthread_mutex_init(&dbg->send_lock, NULL);
thread_mutex_init(&dbg->print_lock);
thread_mutex_init(&dbg->queue_lock);
thread_mutex_init(&dbg->send_lock);
thread_cond_init(&dbg->queue_cond);

if (pthread_create(&dbg->thread, NULL, &hdhomerun_debug_thread_execute, dbg) != 0) {
if (!thread_task_create(&dbg->thread, &hdhomerun_debug_thread_execute, dbg)) {
free(dbg);
return NULL;
}
Expand All @@ -97,7 +97,8 @@ void hdhomerun_debug_destroy(struct hdhomerun_debug_t *dbg)
}

dbg->terminate = true;
pthread_join(dbg->thread, NULL);
thread_cond_signal(&dbg->queue_cond);
thread_task_join(dbg->thread);

if (dbg->prefix) {
free(dbg->prefix);
Expand All @@ -113,9 +114,9 @@ void hdhomerun_debug_destroy(struct hdhomerun_debug_t *dbg)
}

thread_cond_dispose(&dbg->queue_cond);
pthread_mutex_dispose(&dbg->print_lock);
pthread_mutex_dispose(&dbg->queue_lock);
pthread_mutex_dispose(&dbg->send_lock);
thread_mutex_dispose(&dbg->print_lock);
thread_mutex_dispose(&dbg->queue_lock);
thread_mutex_dispose(&dbg->send_lock);
free(dbg);
}

Expand Down Expand Up @@ -143,10 +144,10 @@ void hdhomerun_debug_close(struct hdhomerun_debug_t *dbg, uint64_t timeout)
hdhomerun_debug_flush(dbg, timeout);
}

pthread_mutex_lock(&dbg->send_lock);
thread_mutex_lock(&dbg->send_lock);
hdhomerun_debug_close_internal(dbg);
dbg->connect_delay = 0;
pthread_mutex_unlock(&dbg->send_lock);
thread_mutex_unlock(&dbg->send_lock);
}

void hdhomerun_debug_set_filename(struct hdhomerun_debug_t *dbg, const char *filename)
Expand All @@ -155,15 +156,15 @@ void hdhomerun_debug_set_filename(struct hdhomerun_debug_t *dbg, const char *fil
return;
}

pthread_mutex_lock(&dbg->send_lock);
thread_mutex_lock(&dbg->send_lock);

if (!filename && !dbg->file_name) {
pthread_mutex_unlock(&dbg->send_lock);
thread_mutex_unlock(&dbg->send_lock);
return;
}
if (filename && dbg->file_name) {
if (strcmp(filename, dbg->file_name) == 0) {
pthread_mutex_unlock(&dbg->send_lock);
thread_mutex_unlock(&dbg->send_lock);
return;
}
}
Expand All @@ -179,7 +180,7 @@ void hdhomerun_debug_set_filename(struct hdhomerun_debug_t *dbg, const char *fil
dbg->file_name = strdup(filename);
}

pthread_mutex_unlock(&dbg->send_lock);
thread_mutex_unlock(&dbg->send_lock);
}

void hdhomerun_debug_set_prefix(struct hdhomerun_debug_t *dbg, const char *prefix)
Expand All @@ -188,7 +189,7 @@ void hdhomerun_debug_set_prefix(struct hdhomerun_debug_t *dbg, const char *prefi
return;
}

pthread_mutex_lock(&dbg->print_lock);
thread_mutex_lock(&dbg->print_lock);

if (dbg->prefix) {
free(dbg->prefix);
Expand All @@ -199,7 +200,7 @@ void hdhomerun_debug_set_prefix(struct hdhomerun_debug_t *dbg, const char *prefi
dbg->prefix = strdup(prefix);
}

pthread_mutex_unlock(&dbg->print_lock);
thread_mutex_unlock(&dbg->print_lock);
}

void hdhomerun_debug_enable(struct hdhomerun_debug_t *dbg)
Expand Down Expand Up @@ -242,15 +243,15 @@ void hdhomerun_debug_flush(struct hdhomerun_debug_t *dbg, uint64_t timeout)
timeout = getcurrenttime() + timeout;

while (getcurrenttime() < timeout) {
pthread_mutex_lock(&dbg->queue_lock);
thread_mutex_lock(&dbg->queue_lock);
struct hdhomerun_debug_message_t *message = dbg->queue_head;
pthread_mutex_unlock(&dbg->queue_lock);
thread_mutex_unlock(&dbg->queue_lock);

if (!message) {
return;
}

msleep_approx(10);
msleep_approx(16);
}
}

Expand All @@ -273,6 +274,8 @@ void hdhomerun_debug_vprintf(struct hdhomerun_debug_t *dbg, const char *fmt, va_
return;
}

message->next = NULL;

char *ptr = message->buffer;
char *end = message->buffer + sizeof(message->buffer) - 2;
*end = 0;
Expand All @@ -289,14 +292,14 @@ void hdhomerun_debug_vprintf(struct hdhomerun_debug_t *dbg, const char *fmt, va_
/*
* Debug prefix.
*/
pthread_mutex_lock(&dbg->print_lock);
thread_mutex_lock(&dbg->print_lock);

if (dbg->prefix) {
hdhomerun_sprintf(ptr, end, "%s ", dbg->prefix);
ptr = strchr(ptr, 0);
}

pthread_mutex_unlock(&dbg->print_lock);
thread_mutex_unlock(&dbg->print_lock);

/*
* Message text.
Expand All @@ -314,21 +317,21 @@ void hdhomerun_debug_vprintf(struct hdhomerun_debug_t *dbg, const char *fmt, va_
/*
* Enqueue.
*/
pthread_mutex_lock(&dbg->queue_lock);
thread_mutex_lock(&dbg->queue_lock);

message->next = NULL;
if (dbg->queue_tail) {
dbg->queue_tail->next = message;
dbg->queue_tail = message;
} else {
dbg->queue_head = message;
dbg->queue_tail = message;
}
dbg->queue_tail = message;
dbg->queue_depth++;

pthread_mutex_unlock(&dbg->queue_lock);
bool signal_thread = dbg->enabled || (dbg->queue_depth > 1024 + 100);

if (dbg->enabled) {
thread_mutex_unlock(&dbg->queue_lock);

if (signal_thread) {
thread_cond_signal(&dbg->queue_cond);
}
}
Expand Down Expand Up @@ -393,7 +396,7 @@ static bool hdhomerun_debug_output_message_sock(struct hdhomerun_debug_t *dbg, s

static bool hdhomerun_debug_output_message(struct hdhomerun_debug_t *dbg, struct hdhomerun_debug_message_t *message)
{
pthread_mutex_lock(&dbg->send_lock);
thread_mutex_lock(&dbg->send_lock);

bool ret;
if (dbg->file_name) {
Expand All @@ -402,13 +405,13 @@ static bool hdhomerun_debug_output_message(struct hdhomerun_debug_t *dbg, struct
ret = hdhomerun_debug_output_message_sock(dbg, message);
}

pthread_mutex_unlock(&dbg->send_lock);
thread_mutex_unlock(&dbg->send_lock);
return ret;
}

static void hdhomerun_debug_pop_and_free_message(struct hdhomerun_debug_t *dbg)
{
pthread_mutex_lock(&dbg->queue_lock);
thread_mutex_lock(&dbg->queue_lock);

struct hdhomerun_debug_message_t *message = dbg->queue_head;
dbg->queue_head = message->next;
Expand All @@ -417,20 +420,20 @@ static void hdhomerun_debug_pop_and_free_message(struct hdhomerun_debug_t *dbg)
}
dbg->queue_depth--;

pthread_mutex_unlock(&dbg->queue_lock);
thread_mutex_unlock(&dbg->queue_lock);

free(message);
}

static THREAD_FUNC_PREFIX hdhomerun_debug_thread_execute(void *arg)
static void hdhomerun_debug_thread_execute(void *arg)
{
struct hdhomerun_debug_t *dbg = (struct hdhomerun_debug_t *)arg;

while (!dbg->terminate) {
pthread_mutex_lock(&dbg->queue_lock);
thread_mutex_lock(&dbg->queue_lock);
struct hdhomerun_debug_message_t *message = dbg->queue_head;
uint32_t queue_depth = dbg->queue_depth;
pthread_mutex_unlock(&dbg->queue_lock);
thread_mutex_unlock(&dbg->queue_lock);

if (!message) {
thread_cond_wait(&dbg->queue_cond);
Expand All @@ -454,6 +457,4 @@ static THREAD_FUNC_PREFIX hdhomerun_debug_thread_execute(void *arg)

hdhomerun_debug_pop_and_free_message(dbg);
}

return 0;
}
4 changes: 2 additions & 2 deletions hdhomerun_discover.c
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* hdhomerun_discover.c
*
* Copyright © 2006-2015 Silicondust USA Inc. <www.silicondust.com>.
* Copyright © 2006-2017 Silicondust USA Inc. <www.silicondust.com>.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
Expand Down Expand Up @@ -427,7 +427,7 @@ int hdhomerun_discover_find_devices_v2(struct hdhomerun_discover_t *ds, uint32_t
if (getcurrenttime() >= timeout) {
break;
}
msleep_approx(10);
msleep_approx(16);
continue;
}

Expand Down
55 changes: 53 additions & 2 deletions hdhomerun_os_posix.c
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* hdhomerun_os_posix.c
*
* Copyright © 2006-2016 Silicondust USA Inc. <www.silicondust.com>.
* Copyright © 2006-2017 Silicondust USA Inc. <www.silicondust.com>.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
Expand Down Expand Up @@ -124,8 +124,59 @@ void msleep_minimum(uint64_t ms)
}
}

void pthread_mutex_dispose(pthread_mutex_t *mutex)
struct thread_task_execute_args_t {
thread_task_func_t func;
void *arg;
};

static void *thread_task_execute(void *arg)
{
struct thread_task_execute_args_t *execute_args = (struct thread_task_execute_args_t *)arg;
execute_args->func(execute_args->arg);
free(execute_args);
return NULL;
}

bool thread_task_create(thread_task_t *tid, thread_task_func_t func, void *arg)
{
struct thread_task_execute_args_t *execute_args = (struct thread_task_execute_args_t *)malloc(sizeof(struct thread_task_execute_args_t));
if (!execute_args) {
return false;
}

execute_args->func = func;
execute_args->arg = arg;

if (pthread_create(tid, NULL, thread_task_execute, execute_args) != 0) {
free(execute_args);
return false;
}

return true;
}

void thread_task_join(thread_task_t tid)
{
pthread_join(tid, NULL);
}

void thread_mutex_init(thread_mutex_t *mutex)
{
pthread_mutex_init(mutex, NULL);
}

void thread_mutex_dispose(pthread_mutex_t *mutex)
{
}

void thread_mutex_lock(thread_mutex_t *mutex)
{
pthread_mutex_lock(mutex);
}

void thread_mutex_unlock(thread_mutex_t *mutex)
{
pthread_mutex_unlock(mutex);
}

void thread_cond_init(thread_cond_t *cond)
Expand Down
15 changes: 11 additions & 4 deletions hdhomerun_os_posix.h
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* hdhomerun_os_posix.h
*
* Copyright © 2006-2015 Silicondust USA Inc. <www.silicondust.com>.
* Copyright © 2006-2017 Silicondust USA Inc. <www.silicondust.com>.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
Expand Down Expand Up @@ -41,6 +41,9 @@
#include <pthread.h>

typedef void (*sig_t)(int);
typedef void (*thread_task_func_t)(void *arg);
typedef pthread_t thread_task_t;
typedef pthread_mutex_t thread_mutex_t;

typedef struct {
volatile bool signaled;
Expand All @@ -49,8 +52,6 @@ typedef struct {
} thread_cond_t;

#define LIBHDHOMERUN_API
#define THREAD_FUNC_PREFIX void *
#define THREAD_FUNC_RESULT NULL

#ifdef __cplusplus
extern "C" {
Expand All @@ -61,7 +62,13 @@ extern LIBHDHOMERUN_API uint64_t getcurrenttime(void);
extern LIBHDHOMERUN_API void msleep_approx(uint64_t ms);
extern LIBHDHOMERUN_API void msleep_minimum(uint64_t ms);

extern LIBHDHOMERUN_API void pthread_mutex_dispose(pthread_mutex_t *mutex);
extern LIBHDHOMERUN_API bool thread_task_create(thread_task_t *tid, thread_task_func_t func, void *arg);
extern LIBHDHOMERUN_API void thread_task_join(thread_task_t tid);

extern LIBHDHOMERUN_API void thread_mutex_init(thread_mutex_t *mutex);
extern LIBHDHOMERUN_API void thread_mutex_dispose(thread_mutex_t *mutex);
extern LIBHDHOMERUN_API void thread_mutex_lock(thread_mutex_t *mutex);
extern LIBHDHOMERUN_API void thread_mutex_unlock(thread_mutex_t *mutex);

extern LIBHDHOMERUN_API void thread_cond_init(thread_cond_t *cond);
extern LIBHDHOMERUN_API void thread_cond_dispose(thread_cond_t *cond);
Expand Down
Loading

0 comments on commit f736fab

Please sign in to comment.