GNOME Bugzilla – Bug 747662
Nautilus crashes when "Icons on Desktop" is enabled ("assertion failed: (!container->details->auto_layout)")
Last modified: 2017-02-22 22:29:56 UTC
Nautilus will open briefly, and then crash. I can see icons on the desktop briefly during the time that Nautilus is open. Here is the result of running it from the shell: sh-4.3$ nautilus ** ERROR:nautilus-canvas-container.c:6118:finish_adding_new_icons: assertion failed: (!container->details->auto_layout) Aborted (core dumped) sh-4.3$
Thanks for taking the time to report this. Without a stack trace from the crash it's very hard to determine what caused it. Can you get us a stack trace with debug symbols? Please see https://wiki.gnome.org/Community/GettingInTouch/Bugzilla/GettingTraces for more information on how to do so. When pasting a stack trace in this bug report, please reset the status of this bug report from NEEDINFO to its previous status. Thanks in advance!
I'm not sure what the previous status was, the options given under resolved didn't seem relevant, so I left it as unresolved. I built nautilus and glibc from the Arch Build Service (with strip flag disabled, debug flag enabled in /etc/makepkg.conf), here is the output from gdb: Reading symbols from /usr/bin/nautilus...done. (gdb) set logging file nautilus_trace.log (gdb) set logging on Copying output to nautilus_trace.log. (gdb) thread apply all bt full (gdb) run Starting program: /usr/bin/nautilus [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/lib/libthread_db.so.1". [New Thread 0x7fffe90f4700 (LWP 1418)] [New Thread 0x7fffe98f5700 (LWP 1417)] [New Thread 0x7fffcf77e700 (LWP 1423)] ** ERROR:nautilus-canvas-container.c:6118:finish_adding_new_icons: assertion failed: (!container->details->auto_layout) [New Thread 0x7fffcff7f700 (LWP 1422)] [New Thread 0x7fffd4ed5700 (LWP 1420)] [New Thread 0x7fffe366b700 (LWP 1419)] Program received signal SIGABRT, Aborted. 0x00007ffff3f9f4b7 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55 55 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig); (gdb) continue Continuing. [Thread 0x7fffcff7f700 (LWP 1422) exited] [Thread 0x7fffe90f4700 (LWP 1418) exited] [Thread 0x7fffcf77e700 (LWP 1423) exited] [Thread 0x7fffd4ed5700 (LWP 1420) exited] [Thread 0x7fffe366b700 (LWP 1419) exited] [Thread 0x7fffe98f5700 (LWP 1417) exited] Program terminated with signal SIGABRT, Aborted. The program no longer exists. (gdb) set logging off Done logging to nautilus_trace.log. (gdb) quit
That's still an issue with 3.18 https://retrace.fedoraproject.org/faf/reports/601688/ backtrace from https://bugzilla.redhat.com/show_bug.cgi?id=1246722 "#0 0x00007efccbda7c78 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55 resultvar = 0 pid = 2304 selftid = 2304
+ Trace 235792
We got a similar report on an SLE12 update for the same assert and same backtrace (https://bugzilla.suse.com/show_bug.cgi?id=1001692). In that case it was even worse (since it happened with the nautilus process that handles the desktop, so as soon as the user logged in, the desktop appeared for half a second, then disappeared and the user was returned to gdm) but the problem seems to be the same. I tracked that down to happen because a "nautilus-desktop-icon-size" entry was missing from the [directory] group in ~/.config/nautilus/desktop-metadata (and there was no ~/.local/share/gvfs-metadata/ directory at all). When nautilus_desktop_canvas_view_end_loading finds out there's no "nautilus-desktop-icon-size" entry, it sets "needs_reorganization" to true, which calls nautilus_canvas_view_clean_up_by_name, which calls (indirectly) nautilus_canvas_container_sort, which sets auto_layout to true unconditionally and then calls (indirectly) finish_adding_new_icons, which, if there's any icon having lazy position results in that g_assert(!container->details->auto_layout) being triggered. My proposed patch sets "has_lazy_position" to FALSE for all icons, so all icons are repositioned in finish_adding_new_icons. This only happens when that entry is not in the config file (and nautilus writes that) so it shouldn't happen too often (probably mostly on upgrades).
Created attachment 337045 [details] [review] fix nautilus crash in g_assert(!container->details->auto_layout) when "nautilus-desktop-icon-size" entry is missing in desktop-metadata Patch explained in my previous comment
Review of attachment 337045 [details] [review]: thanks for digging into the code and making this patch! The patch is not done on master though. The code has some irks. The patch accesses the values of the private data of canvas container from the canvas view. Also, if your insight is correct, seems the actual problem is that we don't mark the icons properly as need to be repositioned when the canvas container is marked as auto_layout?
If you download the patch, you'll see that it applies nicely and modifies the right file, nautilus-desktop/nautilus-desktop-canvas-view.c . I tested it before attaching it here. The problem is that I originally did the patch for gnome 3.20.3 (the version in SLE12) and then adapted it manually for openSUSE Tumbleweed/git master, but forgot to modify the Index line in the patch, so it looks like it won't apply, but it will. Yes, the has_lazy_position can be changed on nautilus_canvas_container_sort, next to the place where it sets auto_layout to TRUE but I don't know nautilus code that much (it's the first time I read it), and it looked to me that changing that method would be far more intrusive and could break other scenarios, so I went for the safe path. Do you think setting has_lazy_position to FALSE in nautilus_canvas_container_sort for all icons would be better?
(In reply to Antonio Larrosa from comment #7) > If you download the patch, you'll see that it applies nicely and modifies > the right file, nautilus-desktop/nautilus-desktop-canvas-view.c . I tested > it before attaching it here. The problem is that I originally did the patch > for gnome 3.20.3 (the version in SLE12) and then adapted it manually for > openSUSE Tumbleweed/git master, but forgot to modify the Index line in the > patch, so it looks like it won't apply, but it will. oh I didn't say that because it doesn't apply, but because the patch is using the wrong code style (we changed all the code to a new code style just before 3.22). > > Yes, the has_lazy_position can be changed on nautilus_canvas_container_sort, > next to the place where it sets auto_layout to TRUE but I don't know > nautilus code that much (it's the first time I read it), and it looked to me > that changing that method would be far more intrusive and could break other > scenarios, so I went for the safe path. Do you think setting > has_lazy_position to FALSE in nautilus_canvas_container_sort for all icons > would be better? I'm not so far from you, I read this code maybe 3 times at most. And I agree doing what I propose is more intrusive and could break other scenarios, but it's the right thing to do. Doing the opposite is a recipe for future disaster. Indeed the code needs to be understood to do that, I will try on my side to understand more the code around this code path to avoid any issues we could potentially have.
Ah, sorry, I didn't notice the coding style change. Will you update the patch then? or should I?
(In reply to Antonio Larrosa from comment #9) > Ah, sorry, I didn't notice the coding style change. > > Will you update the patch then? or should I? The patch is not ready not only because of the style change, so if you want to continue working on it I will be happy :)
*** Bug 768303 has been marked as a duplicate of this bug. ***
I am getting this bug in Ubuntu Zesty development. I just temporal solve my problem (or at least it doesn't crash) doing: rm -rf ~/.local/share/gvfs-metadata rm -rf ~/.config/nautilus Which could be a temporal workaround. Which is the status of this bug, the patch would be merged soon? It is pretty critical the fix for a lot users.
Created attachment 346482 [details] [review] canvas-container: fix auto_layout desktop crash If nautilus-desktop-icon-size is missing or changed and if the icons have lazy position set on true then nautilus-desktop crashes. The cause of this crash is because the desktop needs to be reorganized and the icons are marked with has_lazy_position as true while auto_layout is also true. Because of this, the semi_position_icons list is not null, so the desktop crashes, as icons shouldn't be lazily positioned when auto layout is enabled. When there was added support for lazily positioned icons initially, icons where added in the semi_position_icons list only if auto_layout was false. To fix this, add this check again, so that lazy position is used only when auto_layout is false. This fix works because the icons position is set by lay_down_icons in redo_layout just after finish_adding_new_icons, so that desktop icons are not overlapping.
Created attachment 346507 [details] [review] canvas-container: fix auto_layout desktop crash When the metadata of the desktop needs to be recalculated or it's missing, we need to reposition the icons. They have what we call a "lazy position". In order to place them on the desktop we either position them by it's saved position if possible, and if not, moving them as close as possible without overlaping, or by what we call "auto layout", which is basically a perfect grid similar to a regular nautilus window. It's clear from this logic that we do either one way to place them or the other, and both at the same time doesn't make sense. For that we assert we just apply one of this placements algorythms. However, we were hitting this assertion if desktop-metadata was missing (so they have a lazy position) since we need to reorder the icons using the auto layout algorthm but the code was also trying to do the "saved position" algorythm. This issue is introduced by a commit intended to avoid overlapping icons, with id: 40c79aec2d2fdc860eadab9b59682ae0b04d5131. In the initial implementation of "lazy position" support, if "auto layout" was chosen, the icons were repositioned only by the "auto layout" algorithm. To fix this re-add the check that repositions icons only by the "auto layout" algorithm if "auto layout" is chosen.
Review of attachment 346507 [details] [review]: As discussed during this past days, this is the right solution and commit message. Congrats!! This was a hard one to fix and learn the logic under it.
Review of attachment 346507 [details] [review]: Sorry I actually meant: ::: src/nautilus-canvas-container.c @@ +6759,3 @@ { icon = p->data; + if (icon->has_lazy_position && !container->details->auto_layout) instead of this: @@ +6762,3 @@ { assign_icon_position (container, icon); semi_position_icons = g_list_prepend (semi_position_icons, icon); if (asing_icon_position()) { semi_position_icons = g_list_prepend (semi_position_icons, icon); }
Review of attachment 346507 [details] [review]: marking as needs work
Created attachment 346513 [details] [review] canvas-container: fix auto_layout desktop crash When the metadata of the desktop needs to be recalculated or it's missing, we need to reposition the icons. They have what we call a "lazy position". In order to place them on the desktop we either position them by it's saved position if possible, and if not, moving them as close as possible without overlaping, or by what we call "auto layout", which is basically a perfect grid similar to a regular nautilus window. It's clear from this logic that we do either one way to place them or the other, and both at the same time doesn't make sense. For that we assert we just apply one of this placements algorythms. However, we were hitting this assertion if desktop-metadata was missing (so they have a lazy position) since we need to reorder the icons using the auto layout algorthm but the code was also trying to do the "saved position" algorythm. This issue is introduced by a commit intended to avoid overlapping icons, with id: 40c79aec2d2fdc860eadab9b59682ae0b04d5131. In the initial implementation of "lazy position" support, if "auto layout" was chosen, the icons were repositioned only by the "auto layout" algorithm. To fix this re-add the check that repositions icons only by the "auto layout" algorithm if "auto layout" is chosen.
Review of attachment 346513 [details] [review]: now! Congrats!
Comment on attachment 346513 [details] [review] canvas-container: fix auto_layout desktop crash Attachment 346513 [details] pushed as 569f5db - canvas-container: fix auto_layout desktop crash