Skip to content

Commit

Permalink
Fixed: Improved thread-safety
Browse files Browse the repository at this point in the history 
Use mutexes and atomic counters in places identified by Thread
Sanitizer.

Also fixes a potential crash that would occur when at the end of a
frame D_EndFrame() was being called during busy mode, accessing
player data that was being cleared in another thread.
  • Loading branch information
@skyjake
skyjake committed Feb 10, 2017
1 parent 813e97e commit 22d91be
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 69 deletions.
2 changes: 1 addition & 1 deletion doomsday/apps/client/include/ui/inputdevice.h
View file
Expand Up @@ -49,7 +49,7 @@ class InputDevice
* Base class for all controls.
* @todo Attribute a GUID, to simplify bookkeeping. -ds
*/
class Control
class Control : public de::Lockable
{
public:
/// No InputDevice is associated with the control. @ingroup errors
Expand Down
2 changes: 1 addition & 1 deletion doomsday/apps/client/src/audio/base/audiosystem.cpp
View file
Expand Up @@ -974,7 +974,7 @@ DENG2_PIMPL(AudioSystem)
if (!disableRefresh)
{
// Start the refresh thread. It will run until the Sfx module is shut down.
refreshHandle = Sys_StartThread(sfxChannelRefreshThread, nullptr);
refreshHandle = Sys_StartThread(sfxChannelRefreshThread, nullptr, nullptr);
if (!refreshHandle)
{
throw Error("audio::System::initSfx", "Failed to start refresh thread");
Expand Down
6 changes: 3 additions & 3 deletions doomsday/apps/client/src/busyrunner.cpp
View file
Expand Up @@ -40,6 +40,7 @@
#include <de/Log>

#include <QEventLoop>
#include <atomic>

static bool animatedTransitionActive(int busyMode)
{
Expand All @@ -60,7 +61,7 @@ DENG2_PIMPL_NOREF(BusyRunner)
QEventLoop *eventLoop = nullptr;

thread_t busyThread = nullptr;
bool busyDone = false;
std::atomic_bool busyDone { false };
timespan_t busyTime = 0;
bool busyWillAnimateTransition = false;
bool busyWasIgnoringInput = false;
Expand Down Expand Up @@ -190,8 +191,7 @@ BusyRunner::Result BusyRunner::runTask(BusyTask *task)

// Start the busy worker thread, which will process the task in the
// background while we keep the user occupied with nice animations.
d->busyThread = Sys_StartThread(task->worker, task->workerData);
Thread_SetCallback(d->busyThread, busyWorkerTerminated);
d->busyThread = Sys_StartThread(task->worker, task->workerData, busyWorkerTerminated);

DENG_ASSERT(!d->eventLoop);

Expand Down
7 changes: 5 additions & 2 deletions doomsday/apps/client/src/clientapp.cpp
View file
Expand Up @@ -641,9 +641,12 @@ void ClientApp::postFrame()
// frame: it is a good time to update the mouse state.
Mouse_Poll();

if (gx.EndFrame)
if (!BusyMode_Active())
{
gx.EndFrame();
if (gx.EndFrame)
{
gx.EndFrame();
}
}

App_AudioSystem().endFrame();
Expand Down
50 changes: 28 additions & 22 deletions doomsday/apps/client/src/dd_main.cpp
View file
Expand Up @@ -906,10 +906,6 @@ static GameProfile automaticProfile;

static GameProfile const *autoselectGameProfile()
{
// Make sure all files have been found so we can determine which games are playable.
Folder::waitForPopulation();
DoomsdayApp::bundles().waitForEverythingIdentified();

if (auto arg = CommandLine::get().check("-game", 1))
{
String const param = arg.params.first();
Expand Down Expand Up @@ -1013,7 +1009,7 @@ static void assertTypeSizes()
* Engine initialization. Once completed the game loop is ready to be started.
* Called from the app entrypoint function.
*/
static void initialize()
static void initializeWithWindowReady()
{
DENG2_DEBUG_ONLY( assertTypeSizes(); )

Expand All @@ -1034,9 +1030,10 @@ static void initialize()
FR_Init();

// Enter busy mode until startup complete.
Con_InitProgress(200);
//Con_InitProgress(200);
#endif
BusyMode_RunNewTaskWithName(BUSYF_NO_UPLOADS | BUSYF_STARTUP /*| BUSYF_PROGRESS_BAR*/ | (verbose? BUSYF_CONSOLE_OUTPUT : 0),

BusyMode_RunNewTaskWithName(BUSYF_NO_UPLOADS | BUSYF_STARTUP | (verbose? BUSYF_CONSOLE_OUTPUT : 0),
DD_StartupWorker, 0, "Starting up...");

// Engine initialization is complete. Now finish up with the GL.
Expand All @@ -1045,24 +1042,27 @@ static void initialize()
GL_InitRefresh();
App_Resources().clearAllTextureSpecs();
LensFx_Init();
R_InitViewWindow();
UI_LoadFonts();
#endif
App_Resources().initSystemTextures();

#ifdef __CLIENT__
// Do deferred uploads.
Con_SetProgress(100);
#endif
BusyMode_RunNewTaskWithName(BUSYF_STARTUP /*| BUSYF_PROGRESS_BAR*/ | (verbose? BUSYF_CONSOLE_OUTPUT : 0),
//#ifdef __CLIENT__
// // Do deferred uploads.
// Con_SetProgress(100);
//#endif
BusyMode_RunNewTaskWithName(BUSYF_STARTUP | (verbose? BUSYF_CONSOLE_OUTPUT : 0),
DD_DummyWorker, 0, "Buffering...");

//
// Try to locate all required data files for all registered games.
//
#ifdef __CLIENT__
Con_SetProgress(200);
#endif
//#ifdef __CLIENT__
// Con_SetProgress(200);
//#endif

App_Games().checkReadiness();

// Attempt automatic game selection.
if (!CommandLine_Exists("-noautoselect") || isDedicated)
{
Expand Down Expand Up @@ -1240,7 +1240,6 @@ void DD_FinishInitializationAfterWindowReady()
// Now we can get the color transfer table as the window is available.
DisplayMode_SaveOriginalColorTransfer();
# endif

if (!Sys_GLInitialize())
{
App_Error("Error initializing OpenGL.\n");
Expand All @@ -1249,12 +1248,12 @@ void DD_FinishInitializationAfterWindowReady()
{
ClientWindow::main().setTitle(DD_ComposeMainWindowTitle());
}
#endif
#endif // __CLIENT__

// Initialize engine subsystems and initial state.
try
{
initialize();
initializeWithWindowReady();
App::app().notifyStartupComplete();
return;
}
Expand Down Expand Up @@ -1315,6 +1314,11 @@ static dint DD_StartupWorker(void * /*context*/)
//
// Add required engine resource files.
//

// Make sure all files have been found so we can determine which games are playable.
Folder::waitForPopulation();
DoomsdayApp::bundles().waitForEverythingIdentified();

/*String foundPath = App_FileSystem().findPath(de::Uri("doomsday.pk3", RC_PACKAGE),
RLF_DEFAULT, App_ResourceClass(RC_PACKAGE));
foundPath = App_BasePath() / foundPath; // Ensure the path is absolute.
Expand All @@ -1331,6 +1335,10 @@ static dint DD_StartupWorker(void * /*context*/)
File1::tryLoad(File1::LoadAsVanillaFile,
res::DoomsdayPackage::loadableUri(*basePack));
}
else
{
throw Error("DD_StartupWorker", "Failed to find \"net.dengine.legacy.base\" package");
}

// No more files or packages will be loaded in "startup mode" after this point.
App_FileSystem().endStartup();
Expand All @@ -1349,9 +1357,7 @@ static dint DD_StartupWorker(void * /*context*/)

#ifdef __CLIENT__
R_BuildTexGammaLut(texGamma);
UI_LoadFonts();
R_InitSvgs();
R_InitViewWindow();
R_ResetFrameCount();
#endif
//Con_SetProgress(165);
Expand Down Expand Up @@ -1402,7 +1408,7 @@ void DD_CheckTimeDemo()
{
checked = true;
if (CommandLine_CheckWith("-timedemo", 1) || // Timedemo mode.
CommandLine_CheckWith("-playdemo", 1)) // Play-once mode.
CommandLine_CheckWith("-playdemo", 1)) // Play-once mode.
{
Block cmd = String("playdemo %1").arg(CommandLine_Next()).toUtf8();
Con_Execute(CMDS_CMDLINE, cmd.constData(), false, false);
Expand Down
4 changes: 3 additions & 1 deletion doomsday/apps/client/src/gl/gl_defer.cpp
View file
Expand Up @@ -34,6 +34,8 @@
#include "gl/gl_texmanager.h"
#include "gl/texturecontent.h"

#include <atomic>

using namespace de;

#define NUM_RESERVED_TEXTURENAMES 512
Expand Down Expand Up @@ -97,7 +99,7 @@ typedef struct apifunc_s {
static dd_bool deferredInited = false;
static mutex_t deferredMutex;
static DGLuint reservedTextureNames[NUM_RESERVED_TEXTURENAMES];
static volatile int reservedCount = 0;
static std::atomic_int reservedCount;
static volatile deferredtask_t* deferredTaskFirst = NULL;
static volatile deferredtask_t* deferredTaskLast = NULL;

Expand Down
27 changes: 26 additions & 1 deletion doomsday/apps/client/src/ui/axisinputcontrol.cpp
View file
Expand Up @@ -109,27 +109,33 @@ AxisInputControl::~AxisInputControl()

AxisInputControl::Type AxisInputControl::type() const
{
DENG2_GUARD(this);
return d->type;
}

void AxisInputControl::setRawInput(bool yes)
{
DENG2_GUARD(this);
if (yes) d->flags |= IDA_RAW;
else d->flags &= ~IDA_RAW;
else d->flags &= ~IDA_RAW;
}

bool AxisInputControl::isActive() const
{
DENG2_GUARD(this);
return (d->flags & IDA_DISABLED) == 0;
}

bool AxisInputControl::isInverted() const
{
DENG2_GUARD(this);
return (d->flags & IDA_INVERT) != 0;
}

void AxisInputControl::update(timespan_t ticLength)
{
DENG2_GUARD(this);

Smoother_Advance(d->smoother, ticLength);

if (d->type == Stick)
Expand Down Expand Up @@ -169,16 +175,20 @@ void AxisInputControl::update(timespan_t ticLength)

ddouble AxisInputControl::position() const
{
DENG2_GUARD(this);
return d->position;
}

void AxisInputControl::setPosition(ddouble newPosition)
{
DENG2_GUARD(this);
d->position = newPosition;
}

void AxisInputControl::applyRealPosition(dfloat pos)
{
DENG2_GUARD(this);

dfloat const oldRealPos = d->realPosition;
dfloat const transformed = translateRealPosition(pos);

Expand Down Expand Up @@ -206,6 +216,8 @@ void AxisInputControl::applyRealPosition(dfloat pos)

dfloat AxisInputControl::translateRealPosition(dfloat realPos) const
{
DENG2_GUARD(this);

// An inactive axis is always zero.
if (!isActive()) return 0;

Expand Down Expand Up @@ -238,41 +250,50 @@ dfloat AxisInputControl::translateRealPosition(dfloat realPos) const

dfloat AxisInputControl::deadZone() const
{
DENG2_GUARD(this);
return d->deadZone;
}

void AxisInputControl::setDeadZone(dfloat newDeadZone)
{
DENG2_GUARD(this);
d->deadZone = newDeadZone;
}

dfloat AxisInputControl::scale() const
{
DENG2_GUARD(this);
return d->scale;
}

void AxisInputControl::setScale(dfloat newScale)
{
DENG2_GUARD(this);
d->scale = newScale;
}

dfloat AxisInputControl::offset() const
{
DENG2_GUARD(this);
return d->offset;
}

void AxisInputControl::setOffset(dfloat newOffset)
{
DENG2_GUARD(this);
d->offset = newOffset;
}

duint AxisInputControl::time() const
{
DENG2_GUARD(this);
return d->time;
}

String AxisInputControl::description() const
{
DENG2_GUARD(this);

QStringList flags;
if (!isActive()) flags << "disabled";
if (isInverted()) flags << "inverted";
Expand Down Expand Up @@ -301,11 +322,13 @@ String AxisInputControl::description() const

bool AxisInputControl::inDefaultState() const
{
DENG2_GUARD(this);
return d->position == 0; // Centered?
}

void AxisInputControl::reset()
{
DENG2_GUARD(this);
if (d->type == Pointer)
{
// Clear the accumulation.
Expand All @@ -318,6 +341,8 @@ void AxisInputControl::reset()

void AxisInputControl::consoleRegister()
{
DENG2_GUARD(this);

DENG2_ASSERT(hasDevice() && !name().isEmpty());
String controlName = String("input-%1-%2").arg(device().name()).arg(name());

Expand Down

0 comments on commit 22d91be

Please sign in to comment.