| Summary: | if colord service is not available to dbus , gnome-settings-daemon segfault in libcolord | ||
|---|---|---|---|
| Product: | colord | Reporter: | Alban Browaeys <prahal> |
| Component: | client | Assignee: | Richard Hughes <richard> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | minor | ||
| Priority: | medium | ||
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
debug trace showing the two calls and the segfault
libcolord : remove the first call to complete |
||
Created attachment 49852 [details] [review] libcolord : remove the first call to complete Great find, thanks! A couple of questions: 1. In your opionion, do you think it makes more sense to nuke the first g_simple_async_result_complete() or just to move the second g_simple_async_result_complete_in_idle() above the "out:" line? We don't really need to complete in idle if there's no proxy result to wait around for. 2. How did you reproduce this bug? How did you make it so that colord was installed, but not "available"? I'm asking so that I could add something into the self tests so that this kind of bug doesn't happen again. Thanks, Richard. (In reply to comment #2) > 1. In your opionion, do you think it makes more sense to nuke the first > g_simple_async_result_complete() or just to move the second > g_simple_async_result_complete_in_idle() above the "out:" line? We don't really > need to complete in idle if there's no proxy result to wait around for. Moving _idle above the out line looks better, you are right. > 2. How did you reproduce this bug? How did you make it so that colord was > installed, but not "available"? I'm asking so that I could add something into > the self tests so that this kind of bug doesn't happen again. The use case I had was installing calord into a location which I had not told dbus about before my first test. Thus activation always fail. Then I fixed it by includig the location in system-local.conf and session-local.conf . I think the test case could be about g_simple_async_result cb and finish only, ie checking that error is not null after finish . There might not be a need to reproduce the exact use case. (In reply to comment #3) > (In reply to comment #2) > > 1. In your opionion, do you think it makes more sense to nuke the first > > g_simple_async_result_complete() or just to move the second > > g_simple_async_result_complete_in_idle() above the "out:" line? We don't really > > need to complete in idle if there's no proxy result to wait around for. > > Moving _idle above the out line looks better, you are right. If you give me a trivial patch for that, you'll get credit in the NEWS file :-) > I think the test case could be about g_simple_async_result cb and finish only, > ie checking that error is not null after finish . There might not be a need to > reproduce the exact use case. Right, makes sense. Thanks dude. I've adapted your patch and pushed this to master. Thanks. |
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.
Created attachment 49851 [details] debug trace showing the two calls and the segfault There are two calls to g_simple_async complete in cd_client_connect_cb libcolord/cd-client.c on master when connection fails. Ie complete in the client->priv->proxy == NULL case then in the out label there is a call to complete_in_idle. As the cd_client_connect_finish free the error the second calls leads to segfault when the caller of the finish attempt to read the error message.