Bug 77167 - [PATCH] Fix problem with removing non-existent source
Summary: [PATCH] Fix problem with removing non-existent source
Status: RESOLVED FIXED
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: All All
: medium major
Assignee: David Zeuthen (not reading bugmail)
QA Contact: David Zeuthen (not reading bugmail)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-08 08:07 UTC by Lukasz Skalski
Modified: 2014-04-22 20:45 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH] Fix problem with removing non-existent source (706 bytes, text/plain)
2014-04-08 08:07 UTC, Lukasz Skalski
Details
[PATCH v2] Fix problem with removing non-existent source (658 bytes, patch)
2014-04-22 09:20 UTC, Lukasz Skalski
Details | Splinter Review

Description Lukasz Skalski 2014-04-08 08:07:33 UTC
Created attachment 97061 [details]
[PATCH] Fix problem with removing non-existent source

*** Issue ***

root:~> /usr/lib/polkit-1/polkitd
Successfully changed to user polkitd
07:01:19.734: Loading rules from directory /etc/polkit-1/rules.d
07:01:19.742: Loading rules from directory /usr/share/polkit-1/rules.d
07:01:19.756: Finished loading, compiling and executing 2 rules
Entering main event loop
Connected to the system bus
07:01:19.795: Acquired the name org.freedesktop.PolicyKit1 on the system bus
^CHandling SIGINT
Shutting down

(polkitd:9529): GLib-CRITICAL **: Source ID 2 was not found when attempting to remove it
Exiting with code 0


*** Solution ***

The problem is quite old but the message is new because GLib 2.40.0 introduced 
the following change: 

"[..] g_source_remove() will now throw a critical in the case that you try to remove a non-existent source. We expect that there is some code in the wild that will fall afoul of this new critical but considering that we now reuse source IDs, this code is already broken and should probably be fixed."

In polkitd.c file, on_sigint() callback returns FALSE, what means that the source will be removed, so we shouldn't use g_source_remove (sigint_id) after "out" statement.
Comment 1 Colin Walters 2014-04-18 22:07:26 UTC
We still do need to remove the source if we *weren't* interrupted.  Probably make the id a static variable.
Comment 2 Lukasz Skalski 2014-04-22 09:18:41 UTC
(In reply to comment #1)
> We still do need to remove the source if we *weren't* interrupted.  Probably
> make the id a static variable.

Right, so maybe better solution is not remove source in on_sigint() handler - on_sigint() returns TRUE if the source shouldn't be removed.
Comment 3 Lukasz Skalski 2014-04-22 09:20:05 UTC
Created attachment 97736 [details] [review]
[PATCH v2] Fix problem with removing non-existent source
Comment 4 Colin Walters 2014-04-22 20:45:16 UTC
That should work, yes.  I added a topic prefix "polkitd: " and a link to this bug to the commit message and pushed.

Thanks for the patch!


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.