Index: source/tools/atlas/GameInterface/GameLoop.cpp =================================================================== --- source/tools/atlas/GameInterface/GameLoop.cpp +++ source/tools/atlas/GameInterface/GameLoop.cpp @@ -183,9 +183,7 @@ if (msg->GetType() == IMessage::Query) { - // For queries, we need to notify MessagePasserImpl::Query - // that the query has now been processed. - sem_post((sem_t*) static_cast(msg)->m_Semaphore); + msgPasser->NotifyQueryCompleted(); // (msg may have been destructed at this point, so don't use it again) // It's quite possible that the querier is going to do a tiny Index: source/tools/atlas/GameInterface/MessagePasser.h =================================================================== --- source/tools/atlas/GameInterface/MessagePasser.h +++ source/tools/atlas/GameInterface/MessagePasser.h @@ -31,13 +31,16 @@ public: virtual ~MessagePasser() {} + // Takes ownership of IMessage object virtual void Add(IMessage*) = 0; - // takes ownership of IMessage object virtual IMessage* Retrieve() = 0; + // Call from the queried thread to unlock the querying thread. + virtual void NotifyQueryCompleted() = 0; + + // Blocks; caller retains ownership of QueryMessage object virtual void Query(QueryMessage*, void(*timeoutCallback)()) = 0; - // blocks; caller retains ownership of QueryMessage object }; extern MessagePasser* g_MessagePasser; Index: source/tools/atlas/GameInterface/MessagePasserImpl.h =================================================================== --- source/tools/atlas/GameInterface/MessagePasserImpl.h +++ source/tools/atlas/GameInterface/MessagePasserImpl.h @@ -20,8 +20,8 @@ #include "MessagePasser.h" -#include "lib/posix/posix_pthread.h" -#include "ps/CStr.h" +#include +#include #include class MessagePasserImpl : public AtlasMessage::MessagePasser @@ -29,19 +29,20 @@ NONCOPYABLE(MessagePasserImpl); public: MessagePasserImpl(); - ~MessagePasserImpl(); virtual void Add(AtlasMessage::IMessage* msg); virtual AtlasMessage::IMessage* Retrieve(); virtual void Query(AtlasMessage::QueryMessage* qry, void(*timeoutCallback)()); + virtual void NotifyQueryCompleted(); + bool IsEmpty(); void SetTrace(bool t); private: std::mutex m_Mutex; - CStr m_SemaphoreName; - sem_t* m_Semaphore; + std::condition_variable m_ConditionVariable; + bool m_QueryProcessed; std::queue m_Queue; bool m_Trace; }; Index: source/tools/atlas/GameInterface/MessagePasserImpl.cpp =================================================================== --- source/tools/atlas/GameInterface/MessagePasserImpl.cpp +++ source/tools/atlas/GameInterface/MessagePasserImpl.cpp @@ -17,66 +17,13 @@ #include "precompiled.h" -#include - #include "MessagePasserImpl.h" #include "Messages.h" -#include "lib/timer.h" -#include "lib/rand.h" -#include "lib/posix/posix_filesystem.h" - using namespace AtlasMessage; -MessagePasserImpl::MessagePasserImpl() -: m_Trace(false), m_Semaphore(NULL) -{ - int tries = 0; - while (tries++ < 16) // some arbitrary cut-off point to avoid infinite loops - { - static char name[64]; - sprintf_s(name, ARRAY_SIZE(name), "/wfg-atlas-msgpass-%d-%d", - (int)rand(1, 1000), (int)(time(0)%1000)); - sem_t* sem = sem_open(name, O_CREAT | O_EXCL, 0700, 0); - - // This cast should not be necessary, but apparently SEM_FAILED is not - // a value of a pointer type - if (sem == (sem_t*)SEM_FAILED || !sem) - { - int err = errno; - if (err == EEXIST) - { - // Semaphore already exists - try another one - continue; - } - // Otherwise, it's a probably-fatal error - debug_warn(L"sem_open failed"); - break; - } - // Succeeded - use this semaphore - m_Semaphore = sem; - m_SemaphoreName = name; - break; - } - - if (! m_Semaphore) - { - debug_warn(L"Failed to create semaphore for Atlas - giving up"); - // We will probably crash later - maybe we could fall back on sem_init, if this - // ever fails in practice - } -} - -MessagePasserImpl::~MessagePasserImpl() -{ - if (m_Semaphore) - { - // Clean up - sem_close(m_Semaphore); - sem_unlink(m_SemaphoreName.c_str()); - } -} +MessagePasserImpl::MessagePasserImpl() : m_Trace(false), m_QueryProcessed(false) {} void MessagePasserImpl::Add(IMessage* msg) { @@ -102,7 +49,7 @@ { std::lock_guard lock(m_Mutex); - if (! m_Queue.empty()) + if (!m_Queue.empty()) { msg = m_Queue.front(); m_Queue.pop(); @@ -123,64 +70,21 @@ if (m_Trace) debug_printf("%8.3f add query: %s\n", timer_Time(), qry->GetName()); - // Set the semaphore, so we can block until the query has been handled - qry->m_Semaphore = static_cast(m_Semaphore); + std::unique_lock lock(m_Mutex); + m_Queue.push(qry); + m_QueryProcessed = false; + // Wait until the query has been processed. + m_ConditionVariable.wait(lock, [this] { return m_QueryProcessed; }); +} + +void MessagePasserImpl::NotifyQueryCompleted() +{ { std::lock_guard lock(m_Mutex); - m_Queue.push(qry); + m_QueryProcessed = true; } - - // Wait until the query handler has handled the query and called sem_post: - - - // The following code was necessary to avoid deadlock, but it still breaks - // in some cases (e.g. when Atlas issues a query before its event loop starts - // running) and doesn't seem to be the simplest possible solution. - // So currently we're trying to not do anything like that at all, and - // just stop the game making windows (which is what seems (from experience) to - // deadlock things) by overriding ah_display_error. Hopefully it'll work like - // that, and the redundant code below/elsewhere can be removed, but it's - // left in here in case it needs to be reinserted in the future to make it - // work. - // (See http://www.wildfiregames.com/forum/index.php?s=&showtopic=10236&view=findpost&p=174617) - -// // At least on Win32, it is necessary for the UI thread to run its event -// // loop to avoid deadlocking the system (particularly when the game -// // tries to show a dialog box); so timeoutCallback is called whenever we -// // think it's necessary for that to happen. -// -// #if OS_WIN -// // On Win32, use MsgWaitForMultipleObjects, which waits on the semaphore -// // but is also interrupted by incoming Windows-messages. -// // while (0 != (err = sem_msgwait_np(psem))) -// -// while (0 != (err = sem_wait(psem))) -// #else -// // TODO: On non-Win32, I have no idea whether the same problem exists; but -// // it might do, so call the callback every few seconds just in case it helps. -// struct timespec abs_timeout; -// clock_gettime(CLOCK_REALTIME, &abs_timeout); -// abs_timeout.tv_sec += 2; -// while (0 != (err = sem_timedwait(psem, &abs_timeout))) -// #endif - - while (0 != sem_wait(m_Semaphore)) - { - // If timed out, call callback and try again -// if (errno == ETIMEDOUT) -// timeoutCallback(); -// else - // Keep retrying while EINTR, but other errors are probably fatal - if (errno != EINTR) - { - debug_warn(L"Semaphore wait failed"); - return; // (leaks the semaphore) - } - } - - // Clean up - qry->m_Semaphore = NULL; + m_ConditionVariable.notify_all(); } bool MessagePasserImpl::IsEmpty() Index: source/tools/atlas/GameInterface/MessagesSetup.h =================================================================== --- source/tools/atlas/GameInterface/MessagesSetup.h +++ source/tools/atlas/GameInterface/MessagesSetup.h @@ -70,8 +70,6 @@ { Type GetType() const { return IMessage::Query; } void Post(); // defined in ScenarioEditor.cpp - - void* m_Semaphore; // for use by MessagePasser implementations (yay encapsulation) };