- the deadlock was caused by leaking error state through the C-style lumiera_error
- but the reason for the deadlock lies in the »convenience shortcut«
in the Object-Monitor scope guard for entering a wait state immediately.
This function undermines the unlocking-guarantee, when an exception
emanates from within the wait() function itself.
...this function was also ported to the new wrapper,
and can be verified now in a much more succinct way.
''This completes porting of the thread-wrapper''
Since the decision was taken to retain support for this special feature,
and even extend it to allow passing values, the additional functionality
should be documented in the test. Doing so also highlighted subtle problems
with argument binding.
A subtle yet important point: arguments will always be copied into the new thread.
This is a (very sensible) limitation introduced by the C++ standard.
To support seamless use, the thread-wrapper now rewrites the argument types
picked up from the invocation, to prevent passing on a reference type,
which typically ensues when invoking with a variable name. Otherwise
confusing error messages would be emitted from deep within the STD library.
As a further consequence, function signatures involving reference arguments
can no longer be bound (which is desirable; a function to be performed
within a separate thread must either rely on value arguments, or deliberately
use std::ref wrappers to pass references, assuming you know what you're doing)
Now the ThreadWrapper_test offers both
- a really simple usage example
- a comprehensive test to verify that actually the
thread-function is invoked the expected number of times
and that this invocations must have been parallelised
- it is not directly possible to provide a variadic join(args...),
due to overload resolution ambiguities
- as a remedy, simplify the invocation of stringify() for the typical cases,
and provide some frequently used shortcuts
A common usage pattern is to derive from lib::Thread
and then implement the actual thread function as a member function
of this special-Thread-object (possibly also involving other data members)
Provide a simplified invocation for this special case,
also generating the thread-id automatically from the arguments
after all this groundwork, implementing the invocation,
capturing and hand-over of results is simple, and the
thread-wrapper classes became fairly understandable.
This relieves the Thread policy from a lot of technicalities,
while also creating a generally useful tool: the ability to invoke
/anything callable/ (thanks to std::invoke) in a fail-safe way and
transform the exception into an Either type
on second thought, the ability to transport an exception still seems
worthwhile, and can be achieved by some rearrangements in the design.
As preparation, reorganise the design of the Either-wrapper (lib::Result)
VERIFY_ERROR allows to check that an expected except is actually thrown.
The implementation was lazy however;
it just investigated the C-style error flag instead of *really* verifying
that an *lumiera::Exception* with the expected flag was caught.
This discrepancy can be a problem when there is a stray error flag set,
or for some reason the error flag gets cleared before the exception
reaches the top-level catch-block in the test.
- relocate some code into a dedicated translation unit to reduce #includes
- actually set the thread-ID (the old implementation had only a TODO at that point)
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
- cut the ties to the old POSIX-based custom threadpool framework
- remove operations deemed no longer necessary
- sync() obsoleted by the new SyncBarrier
- support anything std::invoke supports
...which is the technique used in the existing Threadpool framwork.
As expected, such a solution is significantly slower than the new
atomics-based implementation. Yet how much slower is still striking.
Timing measurements in concurrent usage situation.
Observed delay is in the order of magnitude of known scheduling leeway;
assuming thus no relevant overhead related to implementation technique
Over time, a collection of microbenchmark helper functions was
extracted from occasional use -- including a variant to perform
parallelised microbenchmarks. While not used beyond sporadic experiments yet,
this framework seems a perfect fit for measuring the SyncBarrier performance.
There is only one catch:
- it uses the old Threadpool + POSIX thread support
- these require the Threadpool service to be started...
- which in turn prohibits using them for libary tests
And last but not least: this setup already requires a barrier.
==> switch the existing microbenchmark setup to c++17 threads preliminarily
(until the thread-wrapper has been reworked).
==> also introduce the new SyncBarrier here immediately
==> use this as a validation test of the setup + SyncBarrier
Using the same building blocks, this operation can be generalised even more,
leading to a much cleaner implementation (also with better type deduction).
The feature actually used here, namely summing up all values,
can then be provided as a convenience shortcut, filling in std::plus
as a default reduction operator.
...first used as part of the test harness;
seemingly this is a generic and generally useful shortcut,
similar to algorithm::reduce (or some kind of fold-left operation)
Intended as replacement for the Mutex/ConditionVar based barrier
built into the exiting Lumiera thread handling framework and used
to ensure safe hand-over of a bound functor into the starting new
thread. The standard requires a comparable guarantee for the C++17
concurrency framework, expressed as a "synchronizes_with" assertion
along the lines of the Atomics framework.
While in most cases dedicated synchronisation is thus not required
anymore when swtiching to C++17, some special extended use cases
remain to be addressed, where the complete initialisation of
further support framework must be ensured.
With C++20 this would be easy to achieve with a std::latch, so we
need a simple workaround for the time being. After consideration of
the typical use case, I am aiming at a middle ground in terms of
performance, by using a yield-wait until satisfying the latch condition.
The investigation for #1279 leads to the following conclusions
- the features and the design of our custom thread-wrapper
almost entirely matches the design chosen meanwhile by the C++ committee
- the implementation provided by the standard library however uses
modern techniques (especially Atomics) and is more precisely worked out
than our custom implementation was.
- we do not need an *active* threadpool with work-assignment,
rather we'll use *active* workers and a *passive* pool,
which was easy to implement based on C++17 features
==> decision to drop our POSIX based custom implementation
and to retrofit the Thread-wrapper as a drop-in replacement
+++ start this refactoring by moving code into the Library
+++ create a copy of the Threadwrapper-code to build and test
the refactorings while the application itself still uses
existing code, until the transition is complete
...which however brings the problem that we can no longer block the destructor
of WorkForce by simply joining on all joinable threads (there is a race
between testing joinable() and invoking join(), which does not tolerate
non-joinable state.
There is a second problem: we need to detect and clean-up terminated workers,
even for just finding out how many workers are still active. Fortunately
doing so also solves the waiting problem in the destructor
While in principle it would be possible (and desirable)
to control worker behaviour exclusively through the Work-Functor's return code,
in practice we must concede that Exceptions can always happen from situations
beyond our control. And while it is necessary for the WorkForce-dtor to
join and block (we can not just pull away the resources from running threads),
the same destructor (when called out of order) must somehow be able
at least to ask the running threads to terminate.
Especially for unit tests this becomes an obnoxious problem -- otherwise
each test failure would cause the test runner to hang.
Thus adding an emergency halt, and also improve setup for tests
with a convenience function to inject a work-function-λ
- investigate consistency guarantees through acquire-release
==> turns out we do not need a fence, but it is tantamount
to have a guard variable and actually load and check
the value to ensure we indeed get a happens-before
- elaborate design of the WorkForce
+ no shared control variables necessary
+ no ability to forcibly shut-down the WorkForce
+ rather, all control will be exerted through the return value
of the Work-Functor
No new functionality, and implementation works as expected.
This test case covers an especially tricky setup, where a calculation
shall be triggered from an external event, while ensuring that the actual
processing can start only after also the regular time-bound scheduling
has taken place (this might be used to prevent an unexpectedly early
external signal to cause writing into an output buffer before the
defined window of data delivery)
...based on the new ability in the ActivityDetector, we can now assign
a custom λ, which deflects back the ctx.post() call into the ActivityLang
instance used for this test case.
While the previously seen behaviour was correct, it was not the call sequence
expected in the real implementation; with this change, on the main-chain
activation the post() now immediately dispatches the notification, which in turn
dispatches the rest of the chain, so that the JobFunctor is indeed
called in this second test case as expected
Up to now, the DiagnosticFun mock in ActivityDetector only
created an EventLog entry on invocation and was able to retunr
a canned result value. Yet for the job invocation scenario test,
it would be desirable to hook-in a λ with a fake implementation
into the ExecutionContext. As a further convenience, the
return value is now default initialised, instead of being
marked as uninitialised until invocation of "returning(val)"
...seems to work, but not really happy with the test setup,
since in real usage the post()-calls would dispatch, while here,
using the ActivityDetector, these calls just log invoation,
and thus the activation is not passed on
