GNOME Bugzilla – Bug 725637
gnome-shell crashed with SIGSEGV in find_output_by_key()
Last modified: 2014-05-15 19:38:47 UTC
Open bug in launchpad.net: https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1275969 "After wake-up the system just displayed this message."
+ Trace 233257
This looks like a problem in a stored display configuration. Can you obtain the monitors.xml file of the affected user, or ask him to try reproduce the bug without that file?
In launchpad.net: https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1275969/comments/8
Created attachment 276012 [details] [review] MetaMonitorConfig: don't keep a previous config with the wrong outputs We can only apply a configuration if its outputs match the connected ones, so discard the current configuration if the set of output changes (for example for hotplug), otherwise we will crash trying to apply the bogus previous configuration.
Created attachment 276013 [details] [review] MetaMonitorConfig: don't always restore the previous config when opening the laptop lid Only do it if the current configuration was actually created as the result of closing the laptop lid.
Created attachment 276014 [details] [review] MetaMonitorConfig: don't always restore the previous config when opening the laptop lid Only do it if the current configuration was actually created as the result of closing the laptop lid. The previous one had a small behavior change, and after some thinking I believe it would have been buggy. This one should work fine.
Review of attachment 276012 [details] [review]: ::: src/backends/meta-monitor-config.c @@ +879,3 @@ Also, we clear the previous configuration if the current one (which is + about to become previous) is stored, of if the current one has a + different outputs. Typos: "of if" => "or if" and "has a different outputs" => "has different outputs" @@ +893,3 @@ + not their modes + */ + if (self->current && config_equal (self->current, config)) If you used if (self->current == NULL || config_equal (self->current, config) here, the config_free() call in the else block could be made unconditional. Could be considered a trade-off with regard to readability though.
Review of attachment 276014 [details] [review]: ::: src/backends/meta-monitor-config.c @@ +1034,2 @@ manager, FALSE); + self->current_is_for_laptop_lid = TRUE; Is this correct when apply_configuration() failed?
(In reply to comment #7) > Review of attachment 276014 [details] [review]: > > ::: src/backends/meta-monitor-config.c > @@ +1034,2 @@ > manager, FALSE); > + self->current_is_for_laptop_lid = TRUE; > > Is this correct when apply_configuration() failed? Mh, no.
Pushed with the suggested fixes. I did not rework the config_equal() check, because it seems more readable this way to me, and I trust compiler optimizations to remove the NULL check anyway. Attachment 276012 [details] pushed as 01cd4b2 - MetaMonitorConfig: don't keep a previous config with the wrong outputs Attachment 276014 [details] pushed as 2ca2c18 - MetaMonitorConfig: don't always restore the previous config when opening the laptop lid
*** Bug 730214 has been marked as a duplicate of this bug. ***
*** Bug 727457 has been marked as a duplicate of this bug. ***