From 112d29d931e99a22afc661765e8511285baedc8c Mon Sep 17 00:00:00 2001 From: Casper Jeukendrup <48658420+cbjeukendrup@users.noreply.github.com> Date: Sat, 26 Mar 2022 17:32:20 +0100 Subject: [PATCH] Enforce that Promise body resolves OR rejects exactly once We now enforce that the body of a promise returns a "Result" struct. Such a struct can only be obtained by calling "resolve" or "reject". Additionally, if you call "resolve" or "reject" without using its result, `Q_REQUIRED_RESULT` will kick in and give you a compiler warning. So you either have to return it (which you should do), or you use it in another way, which is impossible to do unconsciously. The only disadvantage is that you now have to write `return resolve` etc. instead of just `resolve`, but the advantages are bigger. --- .../internal/worker/audiooutputhandler.cpp | 24 +++++----- .../audio/internal/worker/playback.cpp | 4 +- .../audio/internal/worker/playerhandler.cpp | 6 +-- .../audio/internal/worker/trackshandler.cpp | 33 ++++++-------- thirdparty/deto_async/async/promise.h | 45 +++++++++++-------- 5 files changed, 57 insertions(+), 55 deletions(-) diff --git a/src/framework/audio/internal/worker/audiooutputhandler.cpp b/src/framework/audio/internal/worker/audiooutputhandler.cpp index b3e8e85056..b11028e7c8 100644 --- a/src/framework/audio/internal/worker/audiooutputhandler.cpp +++ b/src/framework/audio/internal/worker/audiooutputhandler.cpp @@ -52,16 +52,16 @@ Promise AudioOutputHandler::outputParams(const TrackSequenceI ITrackSequencePtr s = sequence(sequenceId); if (!s) { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } RetVal result = s->audioIO()->outputParams(trackId); if (!result.ret) { - reject(result.ret.code(), result.ret.text()); + return reject(result.ret.code(), result.ret.text()); } - resolve(result.val); + return resolve(result.val); }, AudioThread::ID); } @@ -92,10 +92,10 @@ Promise AudioOutputHandler::masterOutputParams() const ONLY_AUDIO_WORKER_THREAD; IF_ASSERT_FAILED(mixer()) { - reject(static_cast(Err::Undefined), "undefined reference to a mixer"); + return reject(static_cast(Err::Undefined), "undefined reference to a mixer"); } - resolve(mixer()->masterOutputParams()); + return resolve(mixer()->masterOutputParams()); }, AudioThread::ID); } @@ -125,7 +125,7 @@ Promise AudioOutputHandler::availableOutputResources() co Promise::Reject /*reject*/) { ONLY_AUDIO_WORKER_THREAD; - resolve(fxResolver()->resolveAvailableResources()); + return resolve(fxResolver()->resolveAvailableResources()); }, AudioThread::ID); } @@ -138,16 +138,14 @@ Promise AudioOutputHandler::signalChanges(const TrackSequenc ITrackSequencePtr s = sequence(sequenceId); if (!s) { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); - return; + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } if (!s->audioIO()->isHasTrack(trackId)) { - reject(static_cast(Err::InvalidTrackId), "no track"); - return; + return reject(static_cast(Err::InvalidTrackId), "no track"); } - resolve(s->audioIO()->audioSignalChanges(trackId)); + return resolve(s->audioIO()->audioSignalChanges(trackId)); }, AudioThread::ID); } @@ -158,10 +156,10 @@ Promise AudioOutputHandler::masterSignalChanges() const ONLY_AUDIO_WORKER_THREAD; IF_ASSERT_FAILED(mixer()) { - reject(static_cast(Err::Undefined), "undefined reference to a mixer"); + return reject(static_cast(Err::Undefined), "undefined reference to a mixer"); } - resolve(mixer()->masterAudioSignalChanges()); + return resolve(mixer()->masterAudioSignalChanges()); }, AudioThread::ID); } diff --git a/src/framework/audio/internal/worker/playback.cpp b/src/framework/audio/internal/worker/playback.cpp index dafa8992bf..4164b655cd 100644 --- a/src/framework/audio/internal/worker/playback.cpp +++ b/src/framework/audio/internal/worker/playback.cpp @@ -57,7 +57,7 @@ Promise Playback::addSequence() m_sequences.emplace(newId, std::make_shared(newId)); m_sequenceAdded.send(newId); - resolve(std::move(newId)); + return resolve(std::move(newId)); }, AudioThread::ID); } @@ -73,7 +73,7 @@ Promise Playback::sequenceIdList() const result.push_back(pair.first); } - resolve(std::move(result)); + return resolve(std::move(result)); }, AudioThread::ID); } diff --git a/src/framework/audio/internal/worker/playerhandler.cpp b/src/framework/audio/internal/worker/playerhandler.cpp index 956a105efc..e016ee39b9 100644 --- a/src/framework/audio/internal/worker/playerhandler.cpp +++ b/src/framework/audio/internal/worker/playerhandler.cpp @@ -122,16 +122,16 @@ Promise PlayerHandler::setLoop(const TrackSequenceId sequenceId, const mse ITrackSequencePtr s = sequence(sequenceId); if (!s) { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } Ret result = s->player()->setLoop(fromMsec, toMsec); if (!result) { - reject(result.code(), result.text()); + return reject(result.code(), result.text()); } - resolve(result); + return resolve(result); }, AudioThread::ID); } diff --git a/src/framework/audio/internal/worker/trackshandler.cpp b/src/framework/audio/internal/worker/trackshandler.cpp index cc1a95d1d7..465b722730 100644 --- a/src/framework/audio/internal/worker/trackshandler.cpp +++ b/src/framework/audio/internal/worker/trackshandler.cpp @@ -51,9 +51,9 @@ Promise TracksHandler::trackIdList(const TrackSequenceId sequenceId ITrackSequencePtr s = sequence(sequenceId); if (s) { - resolve(s->trackIdList()); + return resolve(s->trackIdList()); } else { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } }, AudioThread::ID); } @@ -66,9 +66,9 @@ Promise TracksHandler::trackName(const TrackSequenceId sequenceId, co ITrackSequencePtr s = sequence(sequenceId); if (s) { - resolve(s->trackName(trackId)); + return resolve(s->trackName(trackId)); } else { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } }, AudioThread::ID); } @@ -84,18 +84,16 @@ Promise TracksHandler::addTrack(const TrackSequenceId sequ ITrackSequencePtr s = sequence(sequenceId); if (!s) { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); - return; + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } RetVal2 result = s->addTrack(trackName, playbackData, params); if (!result.ret) { - reject(result.ret.code(), result.ret.text()); - return; + return reject(result.ret.code(), result.ret.text()); } - resolve(result.val1, result.val2); + return resolve(result.val1, result.val2); }, AudioThread::ID); } @@ -109,17 +107,16 @@ Promise TracksHandler::addTrack(const TrackSequenceId sequ ITrackSequencePtr s = sequence(sequenceId); if (!s) { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); - return; + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } RetVal2 result = s->addTrack(trackName, playbackData, params); if (!result.ret) { - reject(result.ret.code(), result.ret.text()); + return reject(result.ret.code(), result.ret.text()); } - resolve(result.val1, result.val2); + return resolve(result.val1, result.val2); }, AudioThread::ID); } @@ -175,7 +172,7 @@ Promise TracksHandler::availableInputResources() const Promise::Reject /*reject*/) { ONLY_AUDIO_WORKER_THREAD; - resolve(resolver()->resolveAvailableResources()); + return resolve(resolver()->resolveAvailableResources()); }, AudioThread::ID); } @@ -188,18 +185,16 @@ Promise TracksHandler::inputParams(const TrackSequenceId seque ITrackSequencePtr s = sequence(sequenceId); if (!s) { - reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); - return; + return reject(static_cast(Err::InvalidSequenceId), "invalid sequence id"); } RetVal result = s->audioIO()->inputParams(trackId); if (!result.ret) { - reject(result.ret.code(), result.ret.text()); - return; + return reject(result.ret.code(), result.ret.text()); } - resolve(result.val); + return resolve(result.val); }, AudioThread::ID); } diff --git a/thirdparty/deto_async/async/promise.h b/thirdparty/deto_async/async/promise.h index 05f3f2c210..3fb1e9a378 100644 --- a/thirdparty/deto_async/async/promise.h +++ b/thirdparty/deto_async/async/promise.h @@ -14,13 +14,27 @@ template class Promise { public: + // Dummy struct, with the purpose to enforce that the body + // of a Promise resolves OR rejects exactly once + struct Result { + private: + Result() = default; + + friend struct Resolve; + friend struct Reject; + }; struct Resolve { Resolve(Promise _p) : p(_p) {} - void operator ()(const T& ... val) const { p.resolve(val ...); } + Q_REQUIRED_RESULT + Result operator ()(const T& ... val) const + { + p.resolve(val ...); + return {}; + } private: mutable Promise p; @@ -31,32 +45,27 @@ public: Reject(Promise _p) : p(_p) {} - void operator ()(int code, const std::string& msg) const { p.reject(code, msg); } + Q_REQUIRED_RESULT + Result operator ()(int code, const std::string& msg) const + { + p.reject(code, msg); + return {}; + } private: mutable Promise p; }; - template - Promise(Exec exec, const std::thread::id& th = std::this_thread::get_id()) + using Body = std::function; + + Promise(Body body, const std::thread::id& th = std::this_thread::get_id()) { Resolve res(*this); Reject rej(*this); - Async::call(nullptr, [res, rej](Exec exec) mutable { - exec(res, rej); - }, exec, th); - } - - template - Promise(Exec exec) - { - Resolve res(*this); - Reject rej(*this); - - Async::call(nullptr, [res, rej](Exec exec) mutable { - exec(res, rej); - }, exec); + Async::call(nullptr, [res, rej](Body body) mutable { + body(res, rej); + }, body, th); } Promise(const Promise& p)