Thread-Loop Improvements
Curve opened this issue · 0 comments
Rohrkabel needs some heavy improvements regarding the Thread-Loop.
The current support for the thread-loop is a bit unintuitive, namely the following things are not as clear and simple as I'd like them to be:
- Thread Loop
- All created objects (through
core::create(_simple)
) are not safe to be deleted without locking thethread-loop
This means that one has to lock thethread-loop
and then explicitly delete the created object
This is a big caveat and can cause problems when a user is not aware of this, also it's really ugly. - When using any form of
events
the callback for theevent
will be called from thethread-loop
s thread.
This is really problematic because you can'tupdate
from within it.
- All created objects (through
-
Exceptions
Generally, I'd like to avoid throwing exceptions where possible. Currently we throw onpw_proxy_events::error
.
This is problematic when thethread-loop
is used because the exception can then not be properly caught.I'd like to replace the exceptions, maybe we could make the constructor of
proxy
,node
, etc. private and have a static methodcreate
/bind
that returns atl::expected
and constructs the object - It would then return the exception when an error was thrown.
However this would requirecreate
to wait for the object to be successfully bound/created to know if it has an error (i.e.update
the core, which in turn would also complicate things a little)
(implemented in f531d2c)
Possible Solutions
1. Revert the thread-loop
changes and bring back call_safe
.
Pros:
- Would circumvent the
events
issue.
Cons:
call_safe
had it's own caveats and things I didn't like about it- Not as elegant as I'd like it to be
2. Make object constructors (i.e. node
, port
, ...) private and use static method to instantiate the object
And return shared_ptr
that has custom deleter to make sure object is deleted safely.
Pros:
- Would solve the destruction thread-safety issue
- Is somewhat elegant
Cons:
- Would not solve the
events
issue
3. The pipewire-rs
appraoch
See https://pipewire.pages.freedesktop.org/pipewire-rs/pipewire/channel/index.html#examples
Design:
-
Channel
- Can send and receive messages to/from the main-loop
- The exchanged messages can be of any arbitrary type
- In case the channel tries to send a pipewire-object (i.e.
node
,device
,proxy
) we throw a compile-time error (in case the passed object is not aconst-reference
orborrow_ptr
) to make sure the passed object stays in the loop thread
- In case the channel tries to send a pipewire-object (i.e.
- The exchanged messages can be of any arbitrary type
- Can send and receive messages to/from the main-loop
-
Required changes to current code
- Core, Context, Registry should assert that the
main-loop
they get bound to was initialized on the same thread. main_loop::run()
should assert that the creation thread is also the execution thread
- Core, Context, Registry should assert that the
Pros:
- Is pretty elegant
- Solves the
events
issue - Would have no additional impact on the current implementation
Cons:
- None
4. Design all inherently unsafe methods to be async
Design:
- All unsafe methods will ...
- ... (in case the loop is a
thread_loop
) lock the core and bind / create the object, then unlock it and return the not yet evaluated result- we can not wait for the object to be evaluated here because it would not work when called from an
event
- we can not wait for the object to be evaluated here because it would not work when called from an
- ... (in case the loop is a
main_loop
) bind / create the object, trigger anupdate
and return the promise
- ... (in case the loop is a
- The promise can be waited upon (i.e.
promise::get()
,promise::wait()
- However it would
assert
if called from within thethread-loop
s thread.
- However it would
- The promise has a method
promise::error()
that takes a callback which is invoked in case of an error - The promise has a method
promise::then()
that takes a callback which is invoked once the result is ready- The promise will have a
state
which will live until thecallback
has been called - This would ensure that the objects destructor is called from within the
thread-loop
s thread (as long as the object is not movable, copyable, ...) - This would be safe to use even from within the
thread-loop
s thread
- The promise will have a
Pros:
- inherently solves some issues the
call_safe
mechanism had (namely that the object is guaranteed to die within thethread-loop
s thread) - may be a little more intuitive than the
call_safe
approach, as all functions that would require special care would return a promise
Cons:
- Callback Hell
- More complex and while it's a little harder to mess things up with this approach (in comparison to
call_safe
) it's still easy to do things wrong by accident - Will make the code way more complex depending on how it's implemented (would best be implemented with templates, which will in turn bring caveats when used on all major classes)
- Also not the most elegant solution
This issue currently serves as an outlet for my ideas regarding this issue.
I will keep updating this post with ideas that come to mind and will leave it open until I have found a solution that I'm pleased with.
💡 Help is always appreciated! |
---|
In case you're reading this issue and have any ideas or suggestions on how to tackle this problem, feel free to comment! |
Current proposed solution
After some back and forth I think I will settle with solution 3