search

jack-ullery / AppAnvil

Graphical user interface for the AppArmor security module (in-progress)
GNU General Public License v3.0
17 stars 12 forks source link

DispatcherMiddleman has a data race #8

Closed jack-ullery closed 2 years ago

jack-ullery commented 2 years ago

Describe the bug The dispatcher_middleman class is used to communicate between the second thread (that calls commands) and the main GUI thread. Currently, if the second thread calls any update command twice before the main thread is able to run handle_signal, then the data from the first update call is overwritten and never used.

To Reproduce Steps to reproduce the behavior:

  1. Make the unit tests
  2. Run ./dist/appanvil_test
  3. Examine the results of DispatcherMiddlemanTest.UPDATE_PROFILES_PROCESSES_INTERLOCKING

Expected behavior Whenever an update method is called from a second thread of dispatcher_middleman, we expect that whenever the main thread runs handle_signal, it is able to see the data that was saved in the update method.

Instead of using two variables to store data, we should use a blocking_queue to store the data from each update call. Then, in the handle_signal method, we can pop the data from the queue and use this. We may be even able to remove the mutex from dispatcher_middleman using this approach.

Additional context This bug is unlikely to ever present itself in a practical situation, but eventually be fixed. This can happen if the Main thread sends two commands, then waits for a while before finally running handle_signal. With the current state of the code, we rarely send two commands to console_thread in quick succession like this.

jack-ullery commented 2 years ago

This was fixed with commit: e9887b67223a8571d03861f8134308b55da2060b The unit test now reflects that result.