Skip to content

Commit

Permalink
Disable signal handling in threads (see issue #1042)
Browse files Browse the repository at this point in the history
I don't necessarily like having to do these extra calls whenever threads are
spawned (and what happens to signals during the time signal delivery is
disabled?) but it seems to fully fix os.execute.

NOTE: Also disables signals when loading openal, as it or one of its backends
spawns threads internally, and does not disable signals itself.

--HG--
branch : minor
  • Loading branch information
bartbes committed Jul 2, 2017
1 parent bf83629 commit a4a62f3
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 26 deletions.
9 changes: 9 additions & 0 deletions src/modules/audio/openal/Audio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ Audio::Audio()
, poolThread(nullptr)
, distanceModel(DISTANCE_INVERSE_CLAMPED)
{
#if defined(LOVE_LINUX)
// Temporarly block signals, as the thread inherits this mask
love::thread::disableSignals();
#endif

// Passing null for default device.
device = alcOpenDevice(nullptr);

Expand All @@ -116,6 +121,10 @@ Audio::Audio()
if (!alcMakeContextCurrent(context) || alcGetError(device) != ALC_NO_ERROR)
throw love::Exception("Could not make context current.");

#if defined(LOVE_LINUX)
love::thread::reenableSignals();
#endif

#ifdef ALC_EXT_EFX
initializeEFX();

Expand Down
26 changes: 0 additions & 26 deletions src/modules/system/System.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,39 +42,13 @@
#include <spawn.h>
#endif

#if defined(LOVE_LINUX)
static void sigchld_handler(int sig)
{
// Because waitpid can set errno, we need to save it.
auto old = errno;

// Reap whilst there are children waiting to be reaped.
while (waitpid(-1, nullptr, WNOHANG) > 0)
;

errno = old;
}
#endif

namespace love
{
namespace system
{

System::System()
{
#if defined(LOVE_LINUX)
// Enable automatic cleanup of zombie processes
// NOTE: We're using our own handler, instead of SA_NOCLDWAIT because the
// latter breaks wait, and thus os.execute.
// NOTE: This isn't perfect, due to multithreading our SIGCHLD can happen
// on a different thread than the one calling wait(), thus causing a race.
struct sigaction act = {0};
sigemptyset(&act.sa_mask);
act.sa_handler = sigchld_handler;
act.sa_flags = SA_RESTART;
sigaction(SIGCHLD, &act, nullptr);
#endif
}

std::string System::getOS() const
Expand Down
9 changes: 9 additions & 0 deletions src/modules/thread/sdl/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,22 @@ Thread::~Thread()

bool Thread::start()
{
#if defined(LOVE_LINUX)
// Temporarly block signals, as the thread inherits this mask
love::thread::disableSignals();
#endif

Lock l(mutex);
if (running)
return false;
if (thread) // Clean old handle up
SDL_WaitThread(thread, nullptr);
thread = SDL_CreateThread(thread_runner, t->getThreadName(), this);
running = (thread != nullptr);

#if defined(LOVE_LINUX)
love::thread::reenableSignals();
#endif
return running;
}

Expand Down
20 changes: 20 additions & 0 deletions src/modules/thread/threads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

#include "threads.h"

#if defined(LOVE_LINUX)
#include <signal.h>
#endif

namespace love
{
namespace thread
Expand Down Expand Up @@ -153,5 +157,21 @@ Conditional *ConditionalRef::operator->() const
return conditional;
}

#if defined(LOVE_LINUX)
static sigset_t oldset;

void disableSignals()
{
sigset_t newset;
sigfillset(&newset);
pthread_sigmask(SIG_SETMASK, &newset, &oldset);
}

void reenableSignals()
{
pthread_sigmask(SIG_SETMASK, &oldset, nullptr);
}
#endif

} // thread
} // love
5 changes: 5 additions & 0 deletions src/modules/thread/threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ Mutex *newMutex();
Conditional *newConditional();
Thread *newThread(Threadable *t);

#if defined(LOVE_LINUX)
void disableSignals();
void reenableSignals();
#endif

} // thread
} // love

Expand Down

0 comments on commit a4a62f3

Please sign in to comment.