Library: (re)introduce the distinction join / detach
While it would be straight forward from an implementation POV to just expose both variants on the API (as the C++ standard does), it seems prudent to enforce the distinction, and to highlight the auto-detaching behaviour as the preferred standard case. Creating worker threads just for one computation and joining the results seemed like a good idea 30 years ago; today we prefer Futures or asynchronous messaging to achieve similar results in a robust and performant way. ThreadJoinable can come in handy however for writing unit tests, were the controlling master thread has to wait prior to perform verification. So the old design seems well advised in this respect and will be retained
This commit is contained in:
parent
84369201f4
commit
67b010ba7e
5 changed files with 228 additions and 54 deletions
|
|
@ -52,6 +52,11 @@
|
|||
** As a convenience, the destructor blocks for a short timespan of 20ms; a thread running
|
||||
** beyond that grace period will kill the whole application by `std::terminate`.
|
||||
**
|
||||
** For the exceptional case when a supervising thread need to await the termination of
|
||||
** launched threads, a different front-end \ref lib::ThreadJoinable is provided, exposing
|
||||
** the `join()` operation. Such threads *must* be joined however, and thus the destructor
|
||||
** immediately terminates the application in case the thread is still running.
|
||||
**
|
||||
** ## Synchronisation
|
||||
** The C++ standard provides that the end of the `std::thread` constructor _syncs-with_ the
|
||||
** start of the new thread function, and likewise the end of the thread activity _syncs-with_
|
||||
|
|
@ -106,37 +111,36 @@
|
|||
#include <thread>
|
||||
#include <string>
|
||||
#include <utility>
|
||||
#include <chrono>
|
||||
|
||||
|
||||
namespace lib {
|
||||
|
||||
using std::string;
|
||||
|
||||
|
||||
namespace thread {// Thread-wrapper base implementation...
|
||||
|
||||
template<bool autoTerm>
|
||||
class ThreadWrapper
|
||||
: util::MoveOnly
|
||||
{
|
||||
|
||||
template<class FUN, typename...ARGS>
|
||||
void
|
||||
main (string threadID, FUN&& threadFunction, ARGS&& ...args)
|
||||
{
|
||||
markThreadStart (threadID);
|
||||
try {
|
||||
markThreadStart (threadID);
|
||||
// execute the actual operation in this new thread
|
||||
// execute the actual operation in this new thread
|
||||
std::invoke (std::forward<FUN> (threadFunction), std::forward<ARGS> (args)...);
|
||||
}
|
||||
ERROR_LOG_AND_IGNORE (thread, "Thread function")
|
||||
//
|
||||
markThreadEnd (threadID);
|
||||
if (autoTerm)
|
||||
threadImpl_.detach();
|
||||
}
|
||||
|
||||
void
|
||||
markThreadStart (string const& threadID)
|
||||
{
|
||||
string logMsg = util::_Fmt{"Thread '%s' start..."} % threadID;
|
||||
TRACE (thread, "%s", logMsg.c_str());
|
||||
//////////////////////////////////////////////////////////////////////OOO maybe set the the Thread-ID via POSIX ??
|
||||
}
|
||||
|
||||
protected:
|
||||
std::thread threadImpl_;
|
||||
|
|
@ -144,6 +148,11 @@ namespace lib {
|
|||
/** @internal derived classes may create an inactive thread */
|
||||
ThreadWrapper() : threadImpl_{} { }
|
||||
|
||||
~ThreadWrapper()
|
||||
{
|
||||
if (autoTerm and threadImpl_.joinable())
|
||||
waitGracePeriod();
|
||||
}
|
||||
|
||||
public:
|
||||
/** Create a new thread to execute the given operation.
|
||||
|
|
@ -187,6 +196,42 @@ namespace lib {
|
|||
{
|
||||
return threadImpl_.get_id() == std::this_thread::get_id();
|
||||
} // Note: implies get_id() != std::thread::id{} ==> it is running
|
||||
|
||||
private:
|
||||
void
|
||||
markThreadStart (string const& threadID)
|
||||
{
|
||||
string logMsg = util::_Fmt{"Thread '%s' start..."} % threadID;
|
||||
TRACE (thread, "%s", logMsg.c_str());
|
||||
//////////////////////////////////////////////////////////////////////OOO maybe set the the Thread-ID via POSIX ??
|
||||
}
|
||||
|
||||
void
|
||||
markThreadEnd (string const& threadID)
|
||||
{
|
||||
string logMsg = util::_Fmt{"Thread '%s' finished..."} % threadID;
|
||||
TRACE (thread, "%s", logMsg.c_str());
|
||||
}
|
||||
|
||||
void
|
||||
waitGracePeriod() noexcept
|
||||
{
|
||||
using std::chrono::steady_clock;
|
||||
using std::chrono_literals::operator ""ms;
|
||||
|
||||
try {
|
||||
auto start = steady_clock::now();
|
||||
while (threadImpl_.joinable()
|
||||
and steady_clock::now () - start < 20ms
|
||||
)
|
||||
std::this_thread::yield();
|
||||
}
|
||||
ERROR_LOG_AND_IGNORE (thread, "Thread shutdown wait")
|
||||
|
||||
if (threadImpl_.joinable())
|
||||
ALERT (thread, "Thread failed to terminate after grace period. Abort.");
|
||||
// invocation of std::thread dtor will presumably call std::terminate...
|
||||
}
|
||||
};
|
||||
|
||||
}//(End)base implementation.
|
||||
|
|
@ -204,7 +249,7 @@ namespace lib {
|
|||
* `std::terminate` afterwards, should the thread still be active then.
|
||||
*/
|
||||
class Thread
|
||||
: public thread::ThreadWrapper
|
||||
: public thread::ThreadWrapper<true>
|
||||
{
|
||||
|
||||
public:
|
||||
|
|
@ -216,35 +261,29 @@ namespace lib {
|
|||
|
||||
|
||||
|
||||
/**
|
||||
* Variant of the standard case, allowing additionally
|
||||
* to join on the termination of this thread.
|
||||
/************************************************************************//**
|
||||
* Variant of the [standard case](\ref Thread), requiring to wait and `join()`
|
||||
* on the termination of this thread. Useful to collect results calculated
|
||||
* by multiple threads. Note however that the system resources of the thread
|
||||
* are kept around until the `join()` call, and thus also the `bool` conversion
|
||||
* yields `true`, even while the actual operation has already terminated.
|
||||
* @warning Thread must be joined prior to destructor invocation, otherwise
|
||||
* the application is shut down immediately via `std::terminate`.
|
||||
*/
|
||||
class ThreadJoinable
|
||||
: public thread::ThreadWrapper
|
||||
: public thread::ThreadWrapper<false>
|
||||
{
|
||||
public:
|
||||
using ThreadWrapper::ThreadWrapper;
|
||||
|
||||
/** put the caller into a blocking wait until this thread has terminated.
|
||||
* @return token signalling either success or failure.
|
||||
* The caller can find out by invoking `isValid()`
|
||||
* or `maybeThrow()` on this result token
|
||||
*/
|
||||
lib::Result<void>
|
||||
/** put the caller into a blocking wait until this thread has terminated */
|
||||
void
|
||||
join ()
|
||||
{
|
||||
// if (!isValid())
|
||||
// throw error::Logic ("joining on an already terminated thread");
|
||||
//
|
||||
// lumiera_err errorInOtherThread =
|
||||
// "TODO TOD-oh";//lumiera_thread_join (threadHandle_); //////////////////////////////////OOO
|
||||
// threadHandle_ = 0;
|
||||
//
|
||||
// if (errorInOtherThread)
|
||||
// return error::State ("Thread terminated with error", errorInOtherThread);
|
||||
// else
|
||||
// return true;
|
||||
if (not threadImpl_.joinable())
|
||||
throw error::Logic ("joining on an already terminated thread");
|
||||
|
||||
threadImpl_.join();
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -165,7 +165,8 @@ namespace test{
|
|||
TestThread testcase[NUM_THREADS] SIDEEFFECT;
|
||||
|
||||
for (uint i=0; i < NUM_THREADS; ++i)
|
||||
CHECK (testcase[i].join().isValid() );
|
||||
testcase[i].join();
|
||||
// CHECK (testcase[i].join().isValid() ); ////////////////////////////////////////////OOO need a way to pass the verification-Result. Maybe a Future?
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -345,7 +345,7 @@ namespace test {
|
|||
|
||||
~InvocationProducer()
|
||||
{
|
||||
this->join().maybeThrow();
|
||||
this->join(); // .maybeThrow(); /////////////////////////////////////////OOO should detect exceptions in thread explicitly
|
||||
for (auto& id : cmdIDs_)
|
||||
Command::remove (cStr(id));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -65,7 +65,6 @@ namespace test {
|
|||
{
|
||||
simpleUse ();
|
||||
wrongUse ();
|
||||
getError ();
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -111,25 +110,6 @@ namespace test {
|
|||
VERIFY_ERROR(LOGIC, newThread.join() );
|
||||
VERIFY_ERROR(LOGIC, newThread.join() );
|
||||
}
|
||||
|
||||
|
||||
void
|
||||
getError()
|
||||
{
|
||||
ThreadJoinable thread1("test Thread joining-3"
|
||||
, bind (&ThreadWrapperJoin_test::theAction, this, DESTRUCTION_CODE)
|
||||
);
|
||||
|
||||
VERIFY_ERROR(SPECIAL, thread1.join().maybeThrow() );
|
||||
|
||||
|
||||
|
||||
ThreadJoinable thread2("test Thread joining-4"
|
||||
, bind (&ThreadWrapperJoin_test::theAction, this, DESTRUCTION_CODE)
|
||||
);
|
||||
|
||||
CHECK (not thread2.join().isValid()); // can check success without throwing
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -79390,6 +79390,138 @@ Date:   Thu Apr 20 18:53:17 2023 +0200<br/>
|
|||
<arrowlink COLOR="#6e85a1" DESTINATION="ID_750029588" ENDARROW="Default" ENDINCLINATION="-741;56;" ID="Arrow_ID_1885189676" STARTARROW="None" STARTINCLINATION="-489;0;"/>
|
||||
</node>
|
||||
</node>
|
||||
<node BACKGROUND_COLOR="#fdfdcf" COLOR="#ff0000" CREATED="1695677839257" ID="ID_1013931122" MODIFIED="1695677867799" TEXT="Problem: Rückgabewert von join()">
|
||||
<icon BUILTIN="messagebox_warning"/>
|
||||
<node CREATED="1695677870757" ID="ID_1158604113" MODIFIED="1695678100114" TEXT="war für die alte Implementierung kein Problem">
|
||||
<richcontent TYPE="NOTE"><html>
|
||||
<head>
|
||||
|
||||
</head>
|
||||
<body>
|
||||
<p>
|
||||
...da die Implementierung mit einem aktiven Threadpool verbunden war, gab es dort auch eine Managment-Datenstruktur; die Threadpool-Logik hat eine eventuell im Thread noch erkannte Fehlerflag dorthin gespeichert — und join() konnte diese Fehlerflag dann einfach abholen.
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
</richcontent>
|
||||
</node>
|
||||
<node CREATED="1695678101764" ID="ID_411970965" MODIFIED="1695678328883" TEXT="hat bisher aber nur Fehlerflags abgedeckt, keine Exceptions">
|
||||
<richcontent TYPE="NOTE"><html>
|
||||
<head>
|
||||
|
||||
</head>
|
||||
<body>
|
||||
<p>
|
||||
...C kennt ja keine Exceptions — und der Author des Threadpool-Frameworks hielt Exceptions für ein bestenfalls überflüssiges Feature, das es ganz bestimmt nicht rechtfertigt, zusätzlichen Aufwand zu treiben
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
</richcontent>
|
||||
</node>
|
||||
<node CREATED="1695678330517" ID="ID_1924651405" MODIFIED="1695678362369" TEXT="der Rückgabewert wurde bisher über einen »Either-Typ« dargestellt">
|
||||
<richcontent TYPE="NOTE"><html>
|
||||
<head>
|
||||
|
||||
</head>
|
||||
<body>
|
||||
<p>
|
||||
gemeint ist lib::Result
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
</richcontent>
|
||||
</node>
|
||||
<node CREATED="1695679266329" ID="ID_1299395095" MODIFIED="1695679291989" TEXT="das Feature erscheint mir „halbgar“">
|
||||
<node CREATED="1695679293241" ID="ID_782789148" MODIFIED="1695679338571" TEXT="Exceptions könnten strukturierte Diagnose-Info transportieren"/>
|
||||
<node CREATED="1695679339661" ID="ID_292233702" MODIFIED="1695679357836" TEXT="aber genau das wird nicht unterstützt"/>
|
||||
<node CREATED="1695679358706" ID="ID_1430818138" MODIFIED="1695679382617" TEXT="speziell für ReturnValue wird dann eine pseudo-Exception fabriziert"/>
|
||||
<node CREATED="1695679388033" ID="ID_785338496" MODIFIED="1695679526638" TEXT="das Abholen+Auswerfen von Exceptions widerspricht dem Sinn einer join()-Operation">
|
||||
<richcontent TYPE="NOTE"><html>
|
||||
<head>
|
||||
|
||||
</head>
|
||||
<body>
|
||||
<p>
|
||||
...und zwar im Besonderen, wenn man in einer »reaping-loop« alle Kind-Threads ernten möchte; dann würde nur ein Fehler ausgeworfen, und die weiteren Kinder bleiben ungeerntet (und terminieren jetzt, mit der C++14-Lösung sogar das Programm)
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
</richcontent>
|
||||
</node>
|
||||
<node CREATED="1695679527561" ID="ID_1584831418" MODIFIED="1695679550241" TEXT="und das Naheliegendste, nämlich das Abholen von Ergebnissen ist nicht implementiert"/>
|
||||
<node CREATED="1695679555637" ID="ID_1907767140" MODIFIED="1695679621871" TEXT="zu allem Überfluss...">
|
||||
<node CREATED="1695679622825" ID="ID_1467930204" MODIFIED="1695679633789" TEXT="auf Fehlerbehandlung sollte man besser doch keine Logik aufbauen"/>
|
||||
<node CREATED="1695679639113" ID="ID_928695067" MODIFIED="1695679655850" TEXT="und eigentlich sollte man Futures verwenden, und nicht thread::join()"/>
|
||||
</node>
|
||||
</node>
|
||||
<node CREATED="1695679664326" ID="ID_1421960449" MODIFIED="1695679670065" TEXT="bisher nur zwei Verwendungen in Tests">
|
||||
<node CREATED="1695679685747" ID="ID_1845700436" MODIFIED="1695679693430" TEXT="DiagnosticContext_test::verify_heavilyParallelUsage">
|
||||
<node CREATED="1695679695554" ID="ID_1757482897" MODIFIED="1695679724753" TEXT="hier erscheint das Feature sinnvoll"/>
|
||||
<node CREATED="1695679747450" ID="ID_787283000" MODIFIED="1695679769915" TEXT="die eigentliche Verifikation für den Testfall muß noch im Thread erfolgen"/>
|
||||
<node CREATED="1695679770671" ID="ID_740996244" MODIFIED="1695679787345" TEXT="...und sollte dann per join().maybeThrow() in den Test-Thread durchgeworfen werden"/>
|
||||
</node>
|
||||
<node CREATED="1695679805882" ID="ID_1362144709" MODIFIED="1695679814293" TEXT="SessionCommandFunction_test::perform_massivelyParallel">
|
||||
<node CREATED="1695679822509" ID="ID_1148407803" MODIFIED="1695679907392" TEXT="hier wirkt das Feature nur wie Zusatz-Check"/>
|
||||
<node CREATED="1695679908023" ID="ID_1958313345" MODIFIED="1695679933919" TEXT="wenn Fehler irgendwo in der Command-Verarbeitung auftreten würden"/>
|
||||
<node CREATED="1695679934545" ID="ID_1686820062" MODIFIED="1695679967433" TEXT="dann würden zumindest der Erste Fehler druchgeworfen ⟹ test scheitert"/>
|
||||
<node CREATED="1695680196374" ID="ID_1112893340" MODIFIED="1695680217919" TEXT="Naja ... der Test sollte auch so scheitern, wenn die Command-Sequenz gebrochen wäre"/>
|
||||
<node CREATED="1695680219739" ID="ID_1686565092" MODIFIED="1695680272271" TEXT="AAAaber — diese Logik erscheint ziemlich trickreich und undurchsichtig"/>
|
||||
<node CREATED="1695680292241" ID="ID_971197976" MODIFIED="1695680319761" TEXT="und deckt nicht alles Ab, was einem Command so „zustoßen“ kann..."/>
|
||||
<node CREATED="1695680321963" ID="ID_1003295003" MODIFIED="1695680331789" TEXT="insgesamt ehr unglücklich damit...">
|
||||
<font ITALIC="true" NAME="SansSerif" SIZE="12"/>
|
||||
<icon BUILTIN="smily_bad"/>
|
||||
</node>
|
||||
</node>
|
||||
</node>
|
||||
<node BACKGROUND_COLOR="#ccb59b" COLOR="#6e2a38" CREATED="1695680809244" ID="ID_530073405" MODIFIED="1695680823462" TEXT="YAGNI ⟹ das Feature wird aufgegeben">
|
||||
<font ITALIC="true" NAME="SansSerif" SIZE="14"/>
|
||||
<icon BUILTIN="yes"/>
|
||||
<node CREATED="1695680837800" ID="ID_556594754" MODIFIED="1695680932789" TEXT="das ganze Thread-Join-Gedöns hat sich generell als nicht so glücklich herausgestellt">
|
||||
<richcontent TYPE="NOTE"><html>
|
||||
<head>
|
||||
|
||||
</head>
|
||||
<body>
|
||||
<p>
|
||||
Das ist ein klassischer Fall für ein Feature, das zu Beginn so offensichtlich und irre nützlich aussieht — dann aber in der Realität nicht wirklich „fliegt“
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
</richcontent>
|
||||
</node>
|
||||
<node CREATED="1695680933859" ID="ID_9664587" MODIFIED="1695680967629">
|
||||
<richcontent TYPE="NODE"><html>
|
||||
<head>
|
||||
|
||||
</head>
|
||||
<body>
|
||||
<p>
|
||||
heute bevorzugt man für ähnliche Anforderungen das <b>Future / Promise</b> - Konstrukt
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
</richcontent>
|
||||
</node>
|
||||
<node CREATED="1695680969567" ID="ID_854612368" MODIFIED="1695681002718" TEXT="oder man sammelt gleich Ergebnis-Nachrichten in einer Lock-free-Queue"/>
|
||||
<node CREATED="1695681067025" ID="ID_1829988819" MODIFIED="1695681202200" TEXT="Fehler in Threads können im Einzelfall sehr relevant sein — das ist aber nix Generisches">
|
||||
<richcontent TYPE="NOTE"><html>
|
||||
<head>
|
||||
|
||||
</head>
|
||||
<body>
|
||||
<p>
|
||||
Denn tatsächlich sind aufgetretene Fehler dann ehr schon eine Form von Zustand, den man mit einem speziellen Protokoll im Thread-Objekt erfassen und nach dem join() abfragen sollte; so kann man auch die Ergebnisse mehrerer Threads korrekt kombinieren
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
</richcontent>
|
||||
<icon BUILTIN="yes"/>
|
||||
</node>
|
||||
<node CREATED="1695681207655" ID="ID_1998022902" MODIFIED="1695681463386" TEXT="brauche dann aber einen Ersatz für die zwei Tests">
|
||||
<arrowlink COLOR="#b65b7a" DESTINATION="ID_478666570" ENDARROW="Default" ENDINCLINATION="41;-72;" ID="Arrow_ID_1595086726" STARTARROW="None" STARTINCLINATION="-427;24;"/>
|
||||
</node>
|
||||
</node>
|
||||
</node>
|
||||
</node>
|
||||
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1695597475293" ID="ID_1100470933" MODIFIED="1695597482079" TEXT="invokedWithinThread()">
|
||||
<icon BUILTIN="flag-yellow"/>
|
||||
|
|
@ -79819,6 +79951,28 @@ Date:   Thu Apr 20 18:53:17 2023 +0200<br/>
|
|||
<node BACKGROUND_COLOR="#d2beaf" COLOR="#5c4d6e" CREATED="1695512017890" ID="ID_953669300" MODIFIED="1695512023779" TEXT="SyncWaiting_test">
|
||||
<icon BUILTIN="hourglass"/>
|
||||
</node>
|
||||
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1695679685747" ID="ID_1841477437" MODIFIED="1695681430475" TEXT="DiagnosticContext_test">
|
||||
<icon BUILTIN="flag-yellow"/>
|
||||
<node BACKGROUND_COLOR="#fdfdcf" COLOR="#ff0000" CREATED="1695681285804" ID="ID_478666570" MODIFIED="1695681455264" TEXT="Problem: Fehlerübergabe">
|
||||
<linktarget COLOR="#b65b7a" DESTINATION="ID_478666570" ENDARROW="Default" ENDINCLINATION="41;-72;" ID="Arrow_ID_1595086726" SOURCE="ID_1998022902" STARTARROW="None" STARTINCLINATION="-427;24;"/>
|
||||
<icon BUILTIN="messagebox_warning"/>
|
||||
<node CREATED="1695681308242" ID="ID_1949309520" MODIFIED="1695681321659" TEXT="die eigentliche Verifikation muß im Einzelthread erfolgen"/>
|
||||
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1695681322500" ID="ID_428007765" MODIFIED="1695681342909" TEXT="Check-Ergebnis dann wohl per Flag übergeben">
|
||||
<icon BUILTIN="flag-yellow"/>
|
||||
</node>
|
||||
</node>
|
||||
<node CREATED="1695681283396" ID="ID_1116798044" MODIFIED="1695681285000" TEXT="verify_heavilyParallelUsage"/>
|
||||
</node>
|
||||
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1695679805882" ID="ID_62892662" MODIFIED="1695681430475" TEXT="SessionCommandFunction_test">
|
||||
<icon BUILTIN="flag-yellow"/>
|
||||
<node CREATED="1695680321963" ID="ID_1398377573" MODIFIED="1695680331789" TEXT="insgesamt ehr unglücklich damit...">
|
||||
<font ITALIC="true" NAME="SansSerif" SIZE="12"/>
|
||||
<icon BUILTIN="smily_bad"/>
|
||||
</node>
|
||||
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1695681374017" ID="ID_767210660" MODIFIED="1695681418164" TEXT="man könnte/sollte Fehler explizit fangen — als Konsitenzcheck">
|
||||
<icon BUILTIN="flag-yellow"/>
|
||||
</node>
|
||||
</node>
|
||||
</node>
|
||||
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1695394237210" ID="ID_11553358" MODIFIED="1695394251486" TEXT="Applikation umstellen">
|
||||
<icon BUILTIN="flag-yellow"/>
|
||||
|
|
|
|||
Loading…
Reference in a new issue