Skip to content

Commit

Permalink
Fix random seg faults on exit
Browse files Browse the repository at this point in the history
Fairly straightfowrad, programs were randomly seg faulting on exit. This
was happening due to GLUT registering a cleanup function via atexit(),
which then gets called when exit() is called.

The issue happens when exit() is called on a thread other than the GLUT
thread, which leads to the exit() call then attempting to cleanup GLUT
while the other thread is still using it, which randomly leads to seg
faults.

Fixing this is slightly annoying. We cannot stop the GLUT thread, as
the basic GLUT API (which is used on Mac OS) simply does not offer a way
to exit the glutMainLoop() call. Thus the simplest solution is to simply
make sure we call exit() on the GLUT thread, which we can fairly easily
due via the message queue.

That being the case, a new libqb_exit() API was added, which simply
regsiters the GLUT exit message and then waits for the program to end.
The atexit() handler then runs on the GLUT thread and everything works
out fine.

In the future we probably should redo the exit logic a bit so that all
the threads are actually stopped/joined to ensure the exit process is
consistent, however this is good enough for now. Also, there's plenty of
error states which call exit() which I did not address.
  • Loading branch information
mkilgore committed Nov 30, 2022
1 parent 6c288ec commit f7fabda
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/c/libqb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37686,7 +37686,7 @@ void MAIN_LOOP(void *unused) {

snd_un_init();

exit(exit_code);
libqb_exit(exit_code);
}

// used to preserve the previous frame's content for comparison/reuse purposes
Expand Down
5 changes: 5 additions & 0 deletions internal/c/libqb/include/glut-thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ bool libqb_is_glut_up();
// Called at consistent intervals from a GLUT callback
void libqb_process_glut_queue();

// Called to properly exit the program. Necessary because GLUT requires a
// special care to not seg-fault when exiting the program.
void libqb_exit(int);

// These functions perform the same actions as their coresponding glut* functions.
// They tell the GLUT thread to perform the command, returning the result if applicable
void libqb_glut_set_cursor(int style);
Expand All @@ -28,6 +32,7 @@ void libqb_glut_position_window(int x, int y);
void libqb_glut_show_window();
void libqb_glut_hide_window();
void libqb_glut_set_window_title(const char *title);
void libqb_glut_exit_program(int exitcode);

// Convinence macros, exists a function depending on the state of GLUT
#define NEEDS_GLUT(error_result) do { \
Expand Down
9 changes: 9 additions & 0 deletions internal/c/libqb/src/console-only-main-thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,12 @@ void libqb_glut_hide_window() {

void libqb_glut_set_window_title(const char *title) {
}

void libqb_glut_exit_program(int exitcode) {
libqb_exit(exitcode);
}

// Since there's no GLUT thread to deal with we can just exit() like normal
void libqb_exit(int code) {
exit(code);
}
15 changes: 15 additions & 0 deletions internal/c/libqb/src/glut-main-thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,18 @@ void libqb_start_main_thread(int argc, char **argv) {

glutMainLoop();
}

// Due to GLUT making use of cleanup via atexit, we have to call exit() from
// the same thread handling the GLUT logic so that the atexit handler also runs
// from that thread (not doing that can result in a segfault due to using GLUT
// from two threads at the same time).
//
// This is acomplished by simply queuing a GLUT message that calls exit() for us.
void libqb_exit(int exitcode)
{
// If GLUT isn't running then we're free to do the exit() call from here
if (!libqb_is_glut_up())
exit(exitcode);

libqb_glut_exit_program(exitcode);
}
5 changes: 5 additions & 0 deletions internal/c/libqb/src/glut-message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,8 @@ void glut_message_hide_window::execute() {
void glut_message_set_window_title::execute() {
glutSetWindowTitle(newTitle);
}

void glut_message_exit_program::execute() {
exit(exitCode);
}

8 changes: 8 additions & 0 deletions internal/c/libqb/src/glut-message.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ class glut_message_set_window_title : public glut_message {
}
};

class glut_message_exit_program : public glut_message {
public:
int exitCode;
void execute();

glut_message_exit_program(int _exitCode) : glut_message(true), exitCode(_exitCode) { }
};

// Queues a glut_message to be processed. Returns false if the message was not
// queued.
bool libqb_queue_glut_message(glut_message *msg);
Expand Down
10 changes: 10 additions & 0 deletions internal/c/libqb/src/glut-msg-queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,13 @@ void libqb_glut_hide_window() {
void libqb_glut_set_window_title(const char *title) {
libqb_queue_glut_message(new glut_message_set_window_title(title));
}

void libqb_glut_exit_program(int exitcode) {
glut_message_exit_program msg(exitcode);

libqb_queue_glut_message(&msg);
msg.wait_for_response();

// Should never return
exit(exitcode);
}

0 comments on commit f7fabda

Please sign in to comment.