diff --git a/Changes.txt b/Changes.txt index 4f575eab3..bbba0f6cb 100644 --- a/Changes.txt +++ b/Changes.txt @@ -49,6 +49,9 @@ * Fixed bug in autodetecting Genesis controllers. + * Fixed bug with 'hold' events; they are now released a short time after + starting a ROM. + * When starting Stella for the first time, the first ROM selected will determine which path to use by default for subsequent runs. diff --git a/src/common/TimerManager.cxx b/src/common/TimerManager.cxx new file mode 100644 index 000000000..6e08ad489 --- /dev/null +++ b/src/common/TimerManager.cxx @@ -0,0 +1,260 @@ +//============================================================================ +// +// SSSS tt lll lll +// SS SS tt ll ll +// SS tttttt eeee ll ll aaaa +// SSSS tt ee ee ll ll aa +// SS tt eeeeee ll ll aaaaa -- "An Atari 2600 VCS Emulator" +// SS SS tt ee ll ll aa aa +// SSSS ttt eeeee llll llll aaaaa +// +// Copyright (c) 1995-2018 by Bradford W. Mott, Stephen Anthony +// and the Stella Team +// +// See the file "License.txt" for information on usage and redistribution of +// this file, and for a DISCLAIMER OF ALL WARRANTIES. +//============================================================================ + +#include +#include "TimerManager.hxx" + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +TimerManager::TimerManager() + : nextId(no_timer + 1), + queue(), + done(false) +{ +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +TimerManager::~TimerManager() +{ + ScopedLock lock(sync); + + // The worker might not be running + if (worker.joinable()) + { + done = true; + lock.unlock(); + wakeUp.notify_all(); + + // If a timer handler is running, this + // will make sure it has returned before + // allowing any deallocations to happen + worker.join(); + + // Note that any timers still in the queue + // will be destructed properly but they + // will not be invoked + } +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +TimerManager::TimerId TimerManager::addTimer( + millisec msDelay, + millisec msPeriod, + const TFunction& func) +{ + ScopedLock lock(sync); + + // Lazily start thread when first timer is requested + if (!worker.joinable()) + worker = std::thread(&TimerManager::timerThreadWorker, this); + + // Assign an ID and insert it into function storage + auto id = nextId++; + auto iter = active.emplace(id, Timer(id, Clock::now() + Duration(msDelay), + Duration(msPeriod), std::move(func))); + + // Insert a reference to the Timer into ordering queue + Queue::iterator place = queue.emplace(iter.first->second); + + // We need to notify the timer thread only if we inserted + // this timer into the front of the timer queue + bool needNotify = (place == queue.begin()); + + lock.unlock(); + + if (needNotify) + wakeUp.notify_all(); + + return id; +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +bool TimerManager::clear(TimerId id) +{ + ScopedLock lock(sync); + auto i = active.find(id); + return destroy_impl(lock, i, true); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +void TimerManager::clear() +{ + ScopedLock lock(sync); + while (!active.empty()) + destroy_impl(lock, active.begin(), queue.size() == 1); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +std::size_t TimerManager::size() const noexcept +{ + ScopedLock lock(sync); + return active.size(); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +bool TimerManager::empty() const noexcept +{ + ScopedLock lock(sync); + return active.empty(); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +TimerManager& TimerManager::global() +{ + static TimerManager singleton; + return singleton; +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +void TimerManager::timerThreadWorker() +{ + ScopedLock lock(sync); + + while (!done) + { + if (queue.empty()) + { + // Wait for done or work + wakeUp.wait(lock, [this] { return done || !queue.empty(); }); + continue; + } + + auto queueHead = queue.begin(); + Timer& timer = *queueHead; + auto now = Clock::now(); + if (now >= timer.next) + { + queue.erase(queueHead); + + // Mark it as running to handle racing destroy + timer.running = true; + + // Call the handler outside the lock + lock.unlock(); + timer.handler(); + lock.lock(); + + if (timer.running) + { + timer.running = false; + + // If it is periodic, schedule a new one + if (timer.period.count() > 0) + { + timer.next = timer.next + timer.period; + queue.emplace(timer); + } + else + { + // Not rescheduling, destruct it + active.erase(timer.id); + } + } + else + { + // timer.running changed! + // + // Running was set to false, destroy was called + // for this Timer while the callback was in progress + // (this thread was not holding the lock during the callback) + // The thread trying to destroy this timer is waiting on + // a condition variable, so notify it + timer.waitCond->notify_all(); + + // The clearTimer call expects us to remove the instance + // when it detects that it is racing with its callback + active.erase(timer.id); + } + } + else + { + // Wait until the timer is ready or a timer creation notifies + Timestamp next = timer.next; + wakeUp.wait_until(lock, next); + } + } +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +// NOTE: if notify is true, returns with lock unlocked +bool TimerManager::destroy_impl(ScopedLock& lock, TimerMap::iterator i, + bool notify) +{ + assert(lock.owns_lock()); + + if (i == active.end()) + return false; + + Timer& timer = i->second; + if (timer.running) + { + // A callback is in progress for this Timer, + // so flag it for deletion in the worker + timer.running = false; + + // Assign a condition variable to this timer + timer.waitCond.reset(new ConditionVar); + + // Block until the callback is finished + if (std::this_thread::get_id() != worker.get_id()) + timer.waitCond->wait(lock); + } + else + { + queue.erase(timer); + active.erase(i); + + if (notify) + { + lock.unlock(); + wakeUp.notify_all(); + } + } + + return true; +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +// TimerManager::Timer implementation +// + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +TimerManager::Timer::Timer(TimerId tid) + : id(tid), + running(false) +{ +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +TimerManager::Timer::Timer(Timer&& r) noexcept + : id(std::move(r.id)), + next(std::move(r.next)), + period(std::move(r.period)), + handler(std::move(r.handler)), + running(std::move(r.running)) +{ +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +TimerManager::Timer::Timer(TimerId tid, Timestamp tnext, Duration tperiod, + const TFunction& func) noexcept + : id(tid), + next(tnext), + period(tperiod), + handler(std::move(func)), + running(false) +{ +} diff --git a/src/common/TimerManager.hxx b/src/common/TimerManager.hxx new file mode 100644 index 000000000..deff8133f --- /dev/null +++ b/src/common/TimerManager.hxx @@ -0,0 +1,193 @@ +//============================================================================ +// +// SSSS tt lll lll +// SS SS tt ll ll +// SS tttttt eeee ll ll aaaa +// SSSS tt ee ee ll ll aa +// SS tt eeeeee ll ll aaaaa -- "An Atari 2600 VCS Emulator" +// SS SS tt ee ll ll aa aa +// SSSS ttt eeeee llll llll aaaaa +// +// Copyright (c) 1995-2018 by Bradford W. Mott, Stephen Anthony +// and the Stella Team +// +// See the file "License.txt" for information on usage and redistribution of +// this file, and for a DISCLAIMER OF ALL WARRANTIES. +//============================================================================ + +#ifndef TIMER_MANAGER_HXX +#define TIMER_MANAGER_HXX + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "bspf.hxx" + +/** + This class provides a portable periodic/one-shot timer infrastructure + using worker threads and generic C++11 code. + + @author Doug Gale (doug65536) + From "Code Review" + https://codereview.stackexchange.com/questions/127552/portable-periodic-one-shot-timer-thread-follow-up + + Modifications and cleanup for Stella by Stephen Anthony +*/ +class TimerManager +{ + public: + // Each Timer is assigned a unique ID of type TimerId + using TimerId = uInt64; + + // Function object we actually use + using TFunction = std::function; + + // Values that are a large-range millisecond count + using millisec = uInt64; + + // Constructor does not start worker until there is a Timer. + explicit TimerManager(); + + // Destructor is thread safe, even if a timer callback is running. + // All callbacks are guaranteed to have returned before this + // destructor returns. + ~TimerManager(); + + /** + Create a new timer using milliseconds, and add it to the internal queue. + + @param msDelay Callback starts firing this many milliseconds from now + @param msPeriod If non-zero, callback is fired again after this period + @param func The callback to run at the specified interval + + @return Id used to identify the timer for later use + */ + TimerId addTimer(millisec msDelay, millisec msPeriod, const TFunction& func); + + /** + Convenience function; setInterval API like browser javascript. + + Call function every 'period' ms, starting 'period' ms from now. + */ + TimerId setInterval(const TFunction& func, millisec period) { + return addTimer(period, period, std::move(func)); + } + + /** + Convenience function; setTimeout API like browser javascript. + + Call function once 'timeout' ms from now. + */ + TimerId setTimeout(const TFunction& func, millisec timeout) { + return addTimer(timeout, 0, std::move(func)); + } + + /** + Destroy the specified timer. + + Synchronizes with the worker thread if the callback for this timer + is running, which guarantees that the handler for that callback is + not running before clear() returns. + + You are not required to clear any timers. You can forget their + TimerId if you do not need to cancel them. + + The only time you need this is when you want to stop a timer that + has a repetition period, or you want to cancel a timeout that has + not fired yet. + */ + bool clear(TimerId id); + + /** + Destroy all timers, but preserve id uniqueness. + This carefully makes sure every timer is not executing its callback + before destructing it. + */ + void clear(); + + // Peek at current state + std::size_t size() const noexcept; + bool empty() const noexcept; + + // Returns lazily initialized singleton + static TimerManager& global(); + + private: + using Lock = std::mutex; + using ScopedLock = std::unique_lock; + using ConditionVar = std::condition_variable; + + using Clock = std::chrono::steady_clock; + using Timestamp = std::chrono::time_point; + using Duration = std::chrono::milliseconds; + + struct Timer + { + explicit Timer(TimerId id = 0); + Timer(Timer&& r) noexcept; + Timer& operator=(Timer&& r) noexcept; + + Timer(TimerId id, Timestamp next, Duration period, const TFunction& func) noexcept; + + // Never called + Timer(Timer const& r) = delete; + Timer& operator=(Timer const& r) = delete; + + TimerId id; + Timestamp next; + Duration period; + TFunction handler; + + // You must be holding the 'sync' lock to assign waitCond + std::unique_ptr waitCond; + + bool running; + }; + + // Comparison functor to sort the timer "queue" by Timer::next + struct NextActiveComparator + { + bool operator()(Timer const& a, Timer const& b) const noexcept + { + return a.next < b.next; + } + }; + + // Queue is a set of references to Timer objects, sorted by next + using QueueValue = std::reference_wrapper; + using Queue = std::multiset; + using TimerMap = std::unordered_map; + + void timerThreadWorker(); + bool destroy_impl(ScopedLock& lock, TimerMap::iterator i, bool notify); + + // Inexhaustible source of unique IDs + TimerId nextId; + + // The Timer objects are physically stored in this map + TimerMap active; + + // The ordering queue holds references to items in 'active' + Queue queue; + + // One worker thread for an unlimited number of timers is acceptable + // Lazily started when first timer is started + // TODO: Implement auto-stopping the timer thread when it is idle for + // a configurable period. + mutable Lock sync; + ConditionVar wakeUp; + std::thread worker; + bool done; + + // Valid IDs are guaranteed not to be this value + static TimerId constexpr no_timer = TimerId(0); +}; + +#endif // TIMERTHREAD_H diff --git a/src/common/module.mk b/src/common/module.mk index 347065488..362aeefcc 100644 --- a/src/common/module.mk +++ b/src/common/module.mk @@ -15,6 +15,7 @@ MODULE_OBJS := \ src/common/RewindManager.o \ src/common/SoundSDL2.o \ src/common/StateManager.o \ + src/common/TimerManager.o \ src/common/ZipHandler.o \ src/common/AudioQueue.o \ src/common/AudioSettings.o \ diff --git a/src/emucore/EventHandler.cxx b/src/emucore/EventHandler.cxx index 32204d804..cf0923523 100644 --- a/src/emucore/EventHandler.cxx +++ b/src/emucore/EventHandler.cxx @@ -41,6 +41,7 @@ #include "Settings.hxx" #include "Sound.hxx" #include "StateManager.hxx" +#include "TimerManager.hxx" #include "Switches.hxx" #include "M6532.hxx" #include "MouseControl.hxx" @@ -120,6 +121,12 @@ void EventHandler::reset(EventHandlerState state) setState(state); myOSystem.state().reset(); myOSystem.png().setContinuousSnapInterval(0); + + // Reset events almost immediately after starting emulation mode + // We wait a little while (0.5s), since 'hold' events may be present, + // and we want time for the ROM to process them + if(state == EventHandlerState::EMULATION) + myOSystem.timer().setTimeout([&ev = myEvent]() { ev.clear(); }, 500); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/emucore/OSystem.cxx b/src/emucore/OSystem.cxx index 133bcd014..5324d1f2c 100644 --- a/src/emucore/OSystem.cxx +++ b/src/emucore/OSystem.cxx @@ -50,6 +50,7 @@ #include "Random.hxx" #include "SerialPort.hxx" #include "StateManager.hxx" +#include "TimerManager.hxx" #include "Version.hxx" #include "TIA.hxx" #include "DispatchResult.hxx" @@ -140,12 +141,13 @@ bool OSystem::create() myCheatManager->loadCheatDatabase(); #endif - // Create menu and launcher GUI objects + // Create various subsystems (menu and launcher GUI objects, etc) myMenu = make_unique(*this); myCommandMenu = make_unique(*this); myTimeMachine = make_unique(*this); myLauncher = make_unique(*this); myStateManager = make_unique(*this); + myTimerManager = make_unique(); // Create the sound object; the sound subsystem isn't actually // opened until needed, so this is non-blocking (on those systems diff --git a/src/emucore/OSystem.hxx b/src/emucore/OSystem.hxx index 25072c730..1f7a6fac6 100644 --- a/src/emucore/OSystem.hxx +++ b/src/emucore/OSystem.hxx @@ -37,6 +37,7 @@ class SerialPort; class Settings; class Sound; class StateManager; +class TimerManager; class VideoDialog; class EmulationWorker; @@ -170,6 +171,13 @@ class OSystem */ StateManager& state() const { return *myStateManager; } + /** + Get the timer/callback manager of the system. + + @return The timermanager object + */ + TimerManager& timer() const { return *myTimerManager; } + /** Get the PNG handler of the system. @@ -466,6 +474,9 @@ class OSystem // Pointer to the StateManager object unique_ptr myStateManager; + // Pointer to the TimerManager object + unique_ptr myTimerManager; + // PNG object responsible for loading/saving PNG images unique_ptr myPNGLib;