Bug 95513 - [PATCH] Fix a race condition when terminating runaway_killer_thread
Summary: [PATCH] Fix a race condition when terminating runaway_killer_thread
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: David Zeuthen (not reading bugmail)
QA Contact: David Zeuthen (not reading bugmail)
Depends on:
Reported: 2016-05-21 01:10 UTC by Miloslav Trmac
Modified: 2018-04-03 18:29 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

0001-Fix-a-race-condition-when-terminating-runaway_killer.patch (6.87 KB, patch)
2016-05-21 01:11 UTC, Miloslav Trmac
Details | Splinter Review
0001-Fix-a-race-condition-when-terminating-runaway_killer.patch (6.87 KB, patch)
2016-05-21 01:14 UTC, Miloslav Trmac
Details | Splinter Review

Description Miloslav Trmac 2016-05-21 01:10:50 UTC
The code used to call g_main_loop_quit() from the main thread, without
having any guarantee that runaway_killer_thread_func() has even entered
its g_main_loop_run().  If a main loop is not running,
g_main_loop_quit() has no effect.

This could occasionally be reproduced in
test-polkitbackendjsauthority.c, which is creating several very
short-lived PolkitBackendJSAuthority instances.  Real polkitd should not
generally be affected, because it is using a single instance running for
the life of the process ~ for the uptime of the system, enough time to
enter the runaway_killer_thread main loop.
Comment 1 Miloslav Trmac 2016-05-21 01:11:25 UTC
Created attachment 123948 [details] [review]
Comment 2 Miloslav Trmac 2016-05-21 01:14:34 UTC
Created attachment 123949 [details] [review]
Comment 3 Ray Strode 2018-04-02 15:21:45 UTC
i noticed this problem too running the test suite.

I think a more surgical fix would be to start the main loop in the "running" state (e.g. change the FALSE argument to g_main_loop_new to TRUE) and then check g_main_loop_is_running before calling g_main_loop_run.  That was the first possible fix that popped into my head anyway.

But "more surgical" isn't better in this case, I think, though. You're patch simplifies the code and just seems all around better than what I'm proposing above.
Comment 4 Ray Strode [halfline] 2018-04-03 18:29:06 UTC
Seems to work great. pushing it to master.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.