Skip to content

Commit

Permalink
fix(concord-once): make signal safe, and use a proper init counter
Browse files Browse the repository at this point in the history
  • Loading branch information
Anotra authored and lcsmuller committed Feb 1, 2024
1 parent 0fa4373 commit 25aceab
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 50 deletions.
6 changes: 6 additions & 0 deletions include/discord-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ extern "C" {
* @brief Documentation useful when developing or debugging Concord itself
* @{ */

/** @brief dup shutdown fd to listen for ccord_shutdown_async() */
int discord_dup_shutdown_fd(void);

/** @brief Get client from its nested field */
#define CLIENT(ptr, path) CONTAINEROF(ptr, struct discord, path)

Expand Down Expand Up @@ -1204,6 +1207,9 @@ struct discord {
/** the handle for registering and retrieving Discord data */
struct discord_cache cache;

/** fd that gets triggered when ccord_shutdown_async is called */
int shutdown_fd;

struct {
struct discord_timers internal;
struct discord_timers user;
Expand Down
116 changes: 86 additions & 30 deletions src/concord-once.c
Original file line number Diff line number Diff line change
@@ -1,33 +1,40 @@
#include <string.h>
#include <signal.h>
#include <curl/curl.h>
#include <pthread.h>
#include <unistd.h>
#include <poll.h>
#include <sys/ioctl.h>

#include "error.h"
#include "discord-worker.h"

static pthread_mutex_t shutdown_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

/* if set to 1 then client(s) will be disconnected */
int ccord_should_shutdown = 0;
static int shutdown_fds[2] = {
-1,
-1,
};

static int once;
static int init_counter = 0;

void
ccord_shutdown_async(void)
{
pthread_mutex_lock(&shutdown_lock);
ccord_should_shutdown = 1;
pthread_mutex_unlock(&shutdown_lock);
char b = 0;
write(shutdown_fds[1], &b, sizeof b);
}

int
ccord_shutting_down(void)
{
int retval;
pthread_mutex_lock(&shutdown_lock);
retval = ccord_should_shutdown;
pthread_mutex_unlock(&shutdown_lock);
return retval;
struct pollfd pfd = {
.fd = shutdown_fds[0],
.events = POLLIN,
};
if (-1 == shutdown_fds[0]) return 0;
poll(&pfd, 1, 0);
return !!(pfd.revents & POLLIN);
}

#ifdef CCORD_SIGINTCATCH
Expand All @@ -36,43 +43,92 @@ static void
_ccord_sigint_handler(int signum)
{
(void)signum;
fputs("\nSIGINT: Disconnecting running concord client(s) ...\n", stderr);
pthread_mutex_lock(&shutdown_lock);
ccord_should_shutdown = 1;
pthread_mutex_unlock(&shutdown_lock);
const char err_str[] =
"\nSIGINT: Disconnecting running concord client(s) ...\n";
write(STDERR_FILENO, err_str, strlen(err_str));
ccord_shutdown_async();
}
#endif /* CCORD_SIGINTCATCH */

CCORDcode
ccord_global_init()
{
if (once) {
return CCORD_GLOBAL_INIT;
}
else {
pthread_mutex_lock(&lock);
if (0 == init_counter++) {
#ifdef CCORD_SIGINTCATCH
signal(SIGINT, &_ccord_sigint_handler);
#endif
if (0 != curl_global_init(CURL_GLOBAL_DEFAULT)) {
fputs("Couldn't start libcurl's globals\n", stderr);
return CCORD_GLOBAL_INIT;
goto fail_curl_init;
}
if (discord_worker_global_init()) {
if (0 != discord_worker_global_init()) {
fputs("Attempt duplicate global initialization\n", stderr);
return CCORD_GLOBAL_INIT;
goto fail_discord_worker_init;
}
if (0 != pipe(shutdown_fds)) {
fputs("Failed to create shutdown pipe\n", stderr);
goto fail_pipe_init;
}
for (int i = 0; i < 2; i++) {
const int on = 1;
if (0 != ioctl(shutdown_fds[i], FIOCLEX, NULL)) {
fputs("Failed to make shutdown pipe close on execute\n",
stderr);
goto fail_pipe_init;
}
if (0 != ioctl(shutdown_fds[i], FIONBIO, &on)) {
fputs("Failed to make shutdown pipe nonblocking\n", stderr);
goto fail_pipe_init;
}
}
once = 1;
}
pthread_mutex_unlock(&lock);
return CCORD_OK;

fail_pipe_init:
for (int i = 0; i < 2; i++) {
if (-1 != shutdown_fds[i]) {
close(shutdown_fds[i]);
shutdown_fds[i] = -1;
}
}
fail_discord_worker_init:
discord_worker_global_cleanup();
fail_curl_init:
curl_global_cleanup();

init_counter = 0;
pthread_mutex_unlock(&lock);
return CCORD_GLOBAL_INIT;
}

void
ccord_global_cleanup()
{
curl_global_cleanup();
discord_worker_global_cleanup();
once = 0;
pthread_mutex_lock(&shutdown_lock);
ccord_should_shutdown = 0;
pthread_mutex_unlock(&shutdown_lock);
pthread_mutex_lock(&lock);
if (init_counter && 0 == --init_counter) {
curl_global_cleanup();
discord_worker_global_cleanup();
for (int i = 0; i < 2; i++) {
close(shutdown_fds[i]);
shutdown_fds[i] = -1;
}
}
pthread_mutex_unlock(&lock);
}

int
discord_dup_shutdown_fd(void)
{
int fd = -1;
if (-1 == shutdown_fds[0]) return -1;
if (-1 != (fd = dup(shutdown_fds[0]))) {
const int on = 1;
if (0 != ioctl(fd, FIOCLEX, NULL) && 0 != ioctl(fd, FIONBIO, &on)) {
close(fd);
fd = -1;
}
}
return fd;
}
58 changes: 39 additions & 19 deletions src/discord-client.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>

#include "discord.h"
#include "discord-internal.h"
Expand Down Expand Up @@ -50,13 +51,27 @@ _parse_init_string(char *dest, size_t dest_size, const char *src)
return true;
}

static void
_on_shutdown_triggered(struct io_poller *io,
enum io_poller_events events,
void *data)
{
(void)io;
(void)events;
discord_shutdown(data);
}

static void
_discord_init(struct discord *new_client)
{
ccord_global_init();

new_client->io_poller = io_poller_create();
discord_timers_init(&new_client->timers.internal, new_client->io_poller);
discord_timers_init(&new_client->timers.user, new_client->io_poller);
io_poller_socket_add(new_client->io_poller,
new_client->shutdown_fd = discord_dup_shutdown_fd(),
IO_POLLER_IN, _on_shutdown_triggered, new_client);

new_client->workers = calloc(1, sizeof *new_client->workers);
ASSERT_S(!pthread_mutex_init(&new_client->workers->lock, NULL),
Expand Down Expand Up @@ -153,7 +168,8 @@ discord_config_init(const char config_file[])
enable_prefix = ('t' == field.start[f->v.pos]);

if (enable_prefix
&& (f = jsmnf_find(pairs, field.start, "prefix", 6))) {
&& (f = jsmnf_find(pairs, field.start, "prefix", 6)))
{
discord_message_commands_set_prefix(&new_client->commands,
field.start + f->v.pos,
f->v.len);
Expand Down Expand Up @@ -212,28 +228,32 @@ discord_cleanup(struct discord *client)
{
if (!client->is_original) {
_discord_clone_cleanup(client);
free(client);
return;
}
else {
discord_worker_join(client);
discord_rest_cleanup(&client->rest);
discord_gateway_cleanup(&client->gw);
discord_message_commands_cleanup(&client->commands);

close(client->shutdown_fd);
discord_worker_join(client);
discord_rest_cleanup(&client->rest);
discord_gateway_cleanup(&client->gw);
discord_message_commands_cleanup(&client->commands);
#ifdef CCORD_VOICE
discord_voice_connections_cleanup(client);
discord_voice_connections_cleanup(client);
#endif
discord_user_cleanup(&client->self);
if (client->cache.cleanup) client->cache.cleanup(client);
discord_refcounter_cleanup(&client->refcounter);
discord_timers_cleanup(client, &client->timers.user);
discord_timers_cleanup(client, &client->timers.internal);
io_poller_destroy(client->io_poller);
logconf_cleanup(&client->conf);
if (client->token) free(client->token);
pthread_mutex_destroy(&client->workers->lock);
pthread_cond_destroy(&client->workers->cond);
free(client->workers);
}
discord_user_cleanup(&client->self);
if (client->cache.cleanup) client->cache.cleanup(client);
discord_refcounter_cleanup(&client->refcounter);
discord_timers_cleanup(client, &client->timers.user);
discord_timers_cleanup(client, &client->timers.internal);
io_poller_destroy(client->io_poller);
logconf_cleanup(&client->conf);
if (client->token) free(client->token);
pthread_mutex_destroy(&client->workers->lock);
pthread_cond_destroy(&client->workers->cond);
free(client->workers);
free(client);

ccord_global_cleanup();
}

CCORDcode
Expand Down
1 change: 0 additions & 1 deletion src/discord-loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ discord_run(struct discord *client)
CALL_IO_POLLER_POLL(poll_errno, poll_result, client->io_poller,
0);

if (ccord_shutting_down()) discord_shutdown(client);
if (-1 == poll_result) {
/* TODO: handle poll error here */
/* use poll_errno instead of errno */
Expand Down

0 comments on commit 25aceab

Please sign in to comment.