-
-
Notifications
You must be signed in to change notification settings - Fork 718
Open
Description
This issue is a list of concurrency issues that must find solution:
- Issue in
timer_module
:
1.1.
https://github.com/jaagr/polybar/blob/7414e9800879cc3692af5ca217fdb18978ba8b4a/include/modules/meta/timer_module.hpp#L33
Here there is a call to a virtual method however when the class is destroyed by the controller
https://github.com/jaagr/polybar/blob/7414e9800879cc3692af5ca217fdb18978ba8b4a/src/components/controller.cpp#L140-L149
the thread still run and this may leads to a non desired behavior since it is quite dangerous to call virtual methods in a constructor or a destructor (during construction or destruction, the dynamic dispatch considers that the object is of the exact type of current destructor).
Moreover since the vtable changes during the destruction, acceding a virtual method from another thread may resolves in an undefined behavior (I am not really sure about the last sentence, please feel free to verify my statement).
This issue can be solved by adding afinal
to therunning
method. (See Lomadriel@7821065)
Since the final is in an upper class (module
), the dynamic dispatch is not used. The call torunning
method is now statically resolved inside thetimer_module
class and so callingrunning
is equivalent to a call to any non-virtual function.
This issue is also present inevent_module
andinotify_module
,
https://github.com/jaagr/polybar/blob/7414e9800879cc3692af5ca217fdb18978ba8b4a/include/modules/meta/event_module.hpp#L32
https://github.com/jaagr/polybar/blob/7414e9800879cc3692af5ca217fdb18978ba8b4a/include/modules/meta/inotify_module.hpp#L28
1.2.
https://github.com/jaagr/polybar/blob/7414e9800879cc3692af5ca217fdb18978ba8b4a/include/modules/meta/timer_module.hpp#L23-L26
Whenupdate
is called,build
can't be called since the internal state is not protected.
this must be:
const auto check = [&]() -> bool {
std::lock(this->m_updatelock, this->m_buildlock);
std::unique_lock<std::mutex> guard_update(this->m_updatelock, std::adopt_lock);
std::unique_lock<std::mutex> guard_build(this->m_buildlock, std::adopt_lock);
return CAST_MOD(Impl)->update();
};
This problem is also present in event_module
, however the problem can't be solved in this class by using this patch since has_event
from i3 module can block. (The best would be to remove this blocking comportment).
- CRTP problem
Like virtual methods, it is not safe to call method from the derived class in a constructor or a destructor. Like in 1.1, such calls can happen during the destruction of amodule
since the thread is still running. So for example this whole loop is undefined behavior when the destructor is being called if the methods exist in the derived class:
https://github.com/jaagr/polybar/blob/7414e9800879cc3692af5ca217fdb18978ba8b4a/include/modules/meta/timer_module.hpp#L33-L38
I see two solutions to solve this problem:
- adding a
wait_termination
method that callsjoin
on every thread before calling the destructor. This function must be called before callingreset()
in the controller
https://github.com/jaagr/polybar/blob/7414e9800879cc3692af5ca217fdb18978ba8b4a/src/components/controller.cpp#L140-L149
Using this solution also solves 1.1 - moving the join in the most derived class (this is a bad solution since it implies a lot of duplicate code)
signal_emitter
andsignal_receiver
:
The map that stores the signal handlers is not protected (g_signal_receivers
) andemit
,attach
anddetach
can be called at the same time.
I tried to add ashared_timed_mutex
to solve this problem, however since some handlers detach themselves when they receive an event this doesn't work (recursive_mutex
can be a solution but this mutex must be avoided).
A solution could be to lock the mutex, copy the handlers that must receive this event, unlock the mutex, send the event.
Another solution could be to code something likeBoost.Signal2
to do the job.tray_manager
setup(const bar_settings&)
andsettings()
are called at the same time.
Trace:
pulseaudio
adapter
The events queue (m_events) is not protected so for example:wait()
andsubscribe_callback
can be called at the same time.
There are other concurrency issues but some are related to the ones listed here.
patrick96patrick96