mate-desktop / caja

Caja, the file manager for the MATE desktop
https://mate-desktop.org/
Other
265 stars 143 forks source link

(Hidpi) Crash w segfault on confirmation in file conflict dialog #1630

Closed lukefromdc closed 2 years ago

lukefromdc commented 2 years ago

Expected behaviour

When replacing a file by moving or copying from another directory with another file of the same name, thus invoking the file conflict dialog,, clicking "replace" should replace the file and caja should keep running normally

Actual behaviour

Clicking "replace" causes caja to crash instantly, then restart as normal in a MATE session. On opening the file, it is clear the file replacement has succeeded.

Steps to reproduce the behaviour

Copy any short file (short is all I tested) to another directory, then move or copy it back to the original directory

MATE general version

1.26

Package version

locally compiled git master current as of March 22 (no change since then according to git pull)

Linux Distribution

Debian Unstable updated on or after May 15 2022

Link to bugreport of your Distribution (requirement)

None as this is a locally compiled package

lukefromdc commented 2 years ago

I got this backtrace by turning off caja in session>required_components and running caja under gdb in a new session:


Thread 1 "caja" received signal SIGSEGV, Segmentation fault.
0x00007ffff787cc96 in gtk_widget_get_scale_factor () from /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
(gdb) thread apply all bt full

Thread 63 (Thread 0x7fffcf7fe640 (LWP 108856) "pool-caja"):
#0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x00007ffff6f35322 in g_cond_wait_until () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff6eb3671 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007ffff6eb3c52 in g_async_queue_timeout_pop () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#4  0x00007ffff6f0d669 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x00007ffff6f0cd0d in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#6  0x00007ffff7f52d80 in start_thread (arg=0x7fffcf7fe640) at pthread_create.c:481
        ret = <optimized out>
        pd = 0x7fffcf7fe640
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140736674653760, 7480448499796874961, 140737488347038, 140737488347039, 0, 140736674653760, -7480344047404509487, -7480466141809797423}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = 0
#7  0x00007ffff68e176f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 43 (Thread 0x7fffcd6fb640 (LWP 108821) "pool-caja"):
#0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x00007ffff6f3520f in g_cond_wait () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff70a614b in g_io_scheduler_job_send_to_mainloop () at /usr/lib/x86_64-linu--Type <RET> for more, q to quit, c to continue without paging--c
x-gnu/libgio-2.0.so.0
#3  0x00005555556482b4 in move_job (io_job=0x555556f07a40, cancellable=<optimized out>, user_data=0x555556f0a310) at caja-file-operations.c:5243
        job = 0x555556f0a310
        common = 0x555556f0a310
        fallbacks = 0x0
        source_info = {num_files = 0, num_bytes = 0, num_files_since_progress = 0, op = OP_KIND_MOVE}
        transfer_info = {num_files = 0, num_bytes = 0, op = OP_KIND_COPY, last_report_time = 6348032776, last_reported_files_left = 0}
        dest_fs_id = 0x7fffd4007350 "\367K\374+\370\177"
        dest_fs_type = 0x0
        fallback_files = <optimized out>
#4  0x00007ffff70a5ec6 in  () at /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#5  0x00007ffff70d8a44 in  () at /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#6  0x00007ffff6f0d5c4 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#7  0x00007ffff6f0cd0d in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#8  0x00007ffff7f52d80 in start_thread (arg=0x7fffcd6fb640) at pthread_create.c:481
        ret = <optimized out>
        pd = 0x7fffcd6fb640
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140736640038464, 7480448499796874961, 140737488347214, 140737488347215, 0, 140736640038464, -7480339788407564591, -7480466141809797423}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = 0
#9  0x00007ffff68e176f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 5 (Thread 0x7ffff183e640 (LWP 108721) "dconf worker"):
#0  0x00007ffff68d587f in __GI___poll (fds=0x5555558e2f80, nfds=1, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
        sc_ret = -516
        sc_cancel_oldtype = 0
#1  0x00007ffff6ee3b96 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff6ee3c9f in g_main_context_iteration () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007ffff18783bd in  () at /usr/lib/x86_64-linux-gnu/gio/modules/libdconfsettings.so
#4  0x00007ffff6f0cd0d in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x00007ffff7f52d80 in start_thread (arg=0x7ffff183e640) at pthread_create.c:481
        ret = <optimized out>
        pd = 0x7ffff183e640
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737245341248, 7480448499796874961, 140737488344798, 140737488344799, 0, 140737245341248, -7480471006637779247, -7480466141809797423}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = 0
#6  0x00007ffff68e176f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 3 (Thread 0x7ffff28e2640 (LWP 108719) "gdbus"):
#0  0x00007ffff68d587f in __GI___poll (fds=0x7fffe8015570, nfds=5, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
        sc_ret = -516
        sc_cancel_oldtype = 0
#1  0x00007ffff6ee3b96 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff6ee3ed3 in g_main_loop_run () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007ffff7141206 in  () at /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#4  0x00007ffff6f0cd0d in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x00007ffff7f52d80 in start_thread (arg=0x7ffff28e2640) at pthread_create.c:481
        ret = <optimized out>
        pd = 0x7ffff28e2640
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737262790208, 7480448499796874961, 140737488343070, 140737488343071, 0, 140737262790208, -7480477489890912559, -7480466141809797423}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = 0
#6  0x00007ffff68e176f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 2 (Thread 0x7ffff30e3640 (LWP 108718) "gmain"):
#0  0x00007ffff68d587f in __GI___poll (fds=0x5555557dc960, nfds=2, timeout=1493) at ../sysdeps/unix/sysv/linux/poll.c:29
        sc_ret = -516
        sc_cancel_oldtype = 0
#1  0x00007ffff6ee3b96 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff6ee3c9f in g_main_context_iteration () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007ffff6ee3cf1 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#4  0x00007ffff6f0cd0d in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x00007ffff7f52d80 in start_thread (arg=0x7ffff30e3640) at pthread_create.c:481
        ret = <optimized out>
        pd = 0x7ffff30e3640
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737271182912, 7480448499796874961, 140737488342718, 140737488342719, 0, 140737271182912, -7480476389842413871, -7480466141809797423}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = 0
#6  0x00007ffff68e176f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 1 (Thread 0x7ffff3589100 (LWP 108714) "caja"):
#0  0x00007ffff787cc96 in gtk_widget_get_scale_factor () at /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
#1  0x00005555556b0fb7 in file_icons_changed (file=<optimized out>, fcd=0x555556284ac0) at caja-file-conflict-dialog.c:76
        surface = <optimized out>
#2  0x00007ffff6fdb4ef in g_closure_invoke () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#3  0x00007ffff6fed306 in  () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#4  0x00007ffff6ff3689 in g_signal_emit_valist () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#5  0x00007ffff6ff3bb2 in g_signal_emit () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#6  0x000055555565822a in caja_file_emit_changed (file=0x555556620850) at caja-file.c:7753
        link_files = <optimized out>
        p = <optimized out>
        __func__ = "caja_file_emit_changed"
#7  0x000055555563a240 in caja_directory_emit_change_signals (directory=0x555555c14380, changed_files=0x7fffffffe350 = {...}) at caja-directory.c:695
        p = 0x7fffffffe350 = {0x555556620850}
#8  0x00005555556583f5 in caja_file_changed (file=0x555556620850) at caja-file.c:7700
        fake_list = {data=0x555556620850, next=0x0, prev=0x0}
        __func__ = "caja_file_changed"
#9  0x000055555563a698 in caja_directory_notify_files_added (files=<optimized out>) at caja-directory.c:923
        added_lists = Python Exception <class 'gdb.error'>: There is no member named keys.
0x555556e3c8c0
        p = 0x5555559a64a0 = {0x55555590d500, 0x55555590d500}
        parent_directories = Python Exception <class 'gdb.error'>: There is no member named keys.
0x555556e3c120
        file = 0x555556620850
        parent = <optimized out>
        directory = 0x555555c14380
        location = 0x55555590d500
#10 0x000055555563f2c3 in caja_file_changes_consume_changes (consume_all=consume_all@entry=1) at caja-file-changes-queue.c:345
        change = <optimized out>
        additions = 0x5555559a64a0 = {0x55555590d500, 0x55555590d500}
        changes = 0x0
        deletions = 0x0
        moves = 0x0
        position_set_requests = 0x0
        pair = <optimized out>
        position_set = <optimized out>
        chunk_count = 2
        queue = <optimized out>
        __func__ = "caja_file_changes_consume_changes"
#11 0x000055555563fa03 in move_job_done (user_data=0x555556f0a310) at caja-file-operations.c:5165
        job = 0x555556f0a310
#12 0x00007ffff70a5f7f in  () at /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#13 0x00007ffff6ee3894 in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007ffff6ee3bf8 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007ffff6ee3c9f in g_main_context_iteration () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x00007ffff710629d in g_application_run () at /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#17 0x00005555555a63b3 in main (argc=1, argv=0x7fffffffe748) at caja-main.c:271
        retval = <optimized out>
        application = 0x55555577b110
lukefromdc commented 2 years ago

This just started, so we have something invoking (or something IN) gtk_widget_get_scale_factor () not liking a change in some underlying lib. I have locally compiled GTK 3.24.34 and glib 2.72.1, just updated to builds of these today from older versions with no change in this behavior.

Using a 4K monitor, tested both with gtk_window_scaling=2 and gtk_window_scaling=1 with no change in behavior. Will update if laptop with 1080x1920 monitor does not also do this

lukefromdc commented 2 years ago

Just confirmed this does NOT happen on my laptop with a 1080x1920 monitor and window-scaling=1

cwendling commented 2 years ago

Quick glance at the code suggests fcd->details->dest_image is probably invalid when the callback hits. Maybe you could check that when the crash happens by switching to frame 1 and check where does fcd->details->dest_image point. Or if it's really a GTK issue instead, try installing the relevant debug symbols to see whether that'd give more info.

lukefromdc commented 2 years ago

I don't know how to switch to frame 1

lukefromdc commented 2 years ago

It's worth noting that this crash/segfault occurs only on "replace" not "cancel" or "skip" Two things have to be happening at that point: the dialog is being destroyed, and the file is being copied.

I also found that just like on the laptop (never gets this crash) on the desktop, changing window scaling or screen resolution has no effect on the crash, the physical monitor of course remains a 4K monitor. I am wondering if some kind of race condition in the destruction of the dialog is to blame here

lukefromdc commented 2 years ago

Also I just found that Nemo also has this crash, and Nautius (which uses different confirmation dialog) does not.

lukefromdc commented 2 years ago

In testing I found that I could halt the crashes by entirely emptying the function

file_icons_changed in caja-file-conflict-dialog.c but that is not a remedy suitable for general use as I would expect trouble when an emtpy file gets replaced with a good one

lukefromdc commented 2 years ago

Looks like at this point in execution, both

fcd->details->dest_image and fcd->details->src_image are returning bad data or reading out of bounds.

If I hardcode the scale to 2, the backtrace changes to indicating a crash at gtk_image_set_from_surface which also uses these same variables

If I disable everything using one of these variables, the backtrace shows a crash on the other.

Some data must in these variables though(even if it is garbage), as null checking with if (fcd->details->dest_image) and if (fcd->details->src_image){ has no effect on the crash

cwendling commented 2 years ago

I don't know how to switch to frame 1

(gdb) frame 1
(gdb) print fcd
(gdb) print fcd->details
(gdb) print fcd->details->dest_image

But I have a feeling fcd is going to be garbage, although I'm not sure why yet.

Does something like this help?

diff --git a/libcaja-private/caja-file-conflict-dialog.c b/libcaja-private/caja-file-conflict-dialog.c
index 9d86d15c..ff0386a3 100644
--- a/libcaja-private/caja-file-conflict-dialog.c
+++ b/libcaja-private/caja-file-conflict-dialog.c
@@ -370,10 +370,10 @@ file_list_ready_cb (GList *files,
     caja_file_monitor_add (src, fcd, CAJA_FILE_ATTRIBUTES_FOR_ICON);
     caja_file_monitor_add (dest, fcd, CAJA_FILE_ATTRIBUTES_FOR_ICON);

-    details->src_handler_id = g_signal_connect (src, "changed",
-                              G_CALLBACK (file_icons_changed), fcd);
-    details->dest_handler_id = g_signal_connect (dest, "changed",
-                               G_CALLBACK (file_icons_changed), fcd);
+    details->src_handler_id = g_signal_connect_object (src, "changed",
+                              G_CALLBACK (file_icons_changed), fcd, 0);
+    details->dest_handler_id = g_signal_connect_object (dest, "changed",
+                               G_CALLBACK (file_icons_changed), fcd, 0);
 }

 static void
lukefromdc commented 2 years ago

I will have to try that tomorrow, on the road now, omly the desktop his this issue, and it hss to be run while we have sunlight as we are off the grid and rely on solar panels.

lukefromdc commented 2 years ago

Got this on your test:

Thread 1 "caja" received signal SIGSEGV, Segmentation fault.
0x00007ffff787cc96 in gtk_widget_get_scale_factor () from /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
(gdb)  frame 1
#1  0x00005555556b0fb7 in file_icons_changed (file=<optimized out>, fcd=0x555556270a80)
    at caja-file-conflict-dialog.c:78
78      surface = caja_file_get_icon_surface (fcd->details->destination,
(gdb) print fcd->details
$1 = (CajaFileConflictDialogPrivate *) 0x555556270760
(gdb) print fcd->details->dest_image
$2 = 0x555556dc6700
(gdb) 
lukefromdc commented 2 years ago

Still a segfault with your suggested change

Thread 1 "caja" received signal SIGSEGV, Segmentation fault.
0x00005555556b0fa4 in file_icons_changed (file=0x555556616a40, fcd=0x0) at caja-file-conflict-dialog.c:78
78      surface = caja_file_get_icon_surface (fcd->details->destination,
(gdb) frame 1
#1  0x00007ffff6fdb4ef in g_closure_invoke ()
   from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
(gdb)  print fcd->details
No symbol "fcd" in current context.
(gdb) print fcd->details->dest_image
No symbol "fcd" in current context.
(gdb) 

which in turn makes *surface = caja_file_get_icon_surface (fcd->details->destination** no good

cwendling commented 2 years ago
Thread 1 "caja" received signal SIGSEGV, Segmentation fault.
0x00005555556b0fa4 in file_icons_changed (file=0x555556616a40, fcd=0x0) at caja-file-conflict-dialog.c:78
78        surface = caja_file_get_icon_surface (fcd->details->destination,
[…]

Apparently file_icons_changed() is called with a NULL fcd argument somehow… I don't understand how that's even possible. would you have any local change apart from the connect_object() calls?

rbuj commented 2 years ago

First, I think a good approach would be to have two callbacks instead of one, splitting the current callback function into two, one for the source and another for the destination. current cb function:

static void
file_icons_changed (CajaFile *file,
                    CajaFileConflictDialog *fcd)
{
    cairo_surface_t *surface;

    surface = caja_file_get_icon_surface (fcd->details->destination,
                                          CAJA_ICON_SIZE_LARGE,
                                          FALSE,
                                          gtk_widget_get_scale_factor (fcd->details->dest_image),
                                          CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);

    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->dest_image), surface);
    cairo_surface_destroy (surface);

    surface = caja_file_get_icon_surface (fcd->details->source,
                                          CAJA_ICON_SIZE_LARGE,
                                          FALSE,
                                          gtk_widget_get_scale_factor (fcd->details->src_image),
                                          CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);

    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->src_image), surface);
    cairo_surface_destroy (surface);
}
cwendling commented 2 years ago

@rbuj that would probably make sense as there's no point in updating both if only one changed, but that shouldn't be the root issue here I think.

rbuj commented 2 years ago
diff --git a/libcaja-private/caja-file-conflict-dialog.c b/libcaja-private/caja-file-conflict-dialog.c
index 9d86d15c..233f47ae 100644
--- a/libcaja-private/caja-file-conflict-dialog.c
+++ b/libcaja-private/caja-file-conflict-dialog.c
@@ -68,27 +68,18 @@ G_DEFINE_TYPE_WITH_PRIVATE (CajaFileConflictDialog,
                             GTK_TYPE_DIALOG);

 static void
-file_icons_changed (CajaFile *file,
-                    CajaFileConflictDialog *fcd)
+file_icons_changed (CajaFile  *file,
+                    GtkWidget *widget)
 {
     cairo_surface_t *surface;

-    surface = caja_file_get_icon_surface (fcd->details->destination,
+    surface = caja_file_get_icon_surface (file,
                                           CAJA_ICON_SIZE_LARGE,
                                           FALSE,
-                                          gtk_widget_get_scale_factor (fcd->details->dest_image),
+                                          gtk_widget_get_scale_factor (widget),
                                           CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);

-    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->dest_image), surface);
-    cairo_surface_destroy (surface);
-
-    surface = caja_file_get_icon_surface (fcd->details->source,
-                                          CAJA_ICON_SIZE_LARGE,
-                                          FALSE,
-                                          gtk_widget_get_scale_factor (fcd->details->src_image),
-                                          CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);
-
-    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->src_image), surface);
+    gtk_image_set_from_surface (GTK_IMAGE (widget), surface);
     cairo_surface_destroy (surface);
 }

@@ -371,9 +362,11 @@ file_list_ready_cb (GList *files,
     caja_file_monitor_add (dest, fcd, CAJA_FILE_ATTRIBUTES_FOR_ICON);

     details->src_handler_id = g_signal_connect (src, "changed",
-                              G_CALLBACK (file_icons_changed), fcd);
+                                                G_CALLBACK (file_icons_changed),
+                                                fcd->details->src_image);
     details->dest_handler_id = g_signal_connect (dest, "changed",
-                               G_CALLBACK (file_icons_changed), fcd);
+                                                 G_CALLBACK (file_icons_changed),
+                                                 fcd->details->dest_image);
 }

 static void
rbuj commented 2 years ago

PR #1632

cwendling commented 2 years ago

@lukefromdc I'm nor sure if that could account for the issue, but Valgrind pointed out to me an issue, it looks like the scale in the icon cache key is no properly populated. Could you give this a shot?

diff --git a/libcaja-private/caja-icon-info.c b/libcaja-private/caja-icon-info.c
index cf690a6e..ee52e4bd 100644
--- a/libcaja-private/caja-icon-info.c
+++ b/libcaja-private/caja-icon-info.c
@@ -307,6 +307,7 @@ icon_key_new (GIcon *icon,

     key = g_slice_new (IconKey);
     key->icon = g_object_ref (icon);
+    key->scale = scale;
     key->size = size;

     return key;
lukefromdc commented 2 years ago

Still got a crash with the last patch posted here applied. The again hardcoded scale to 2 to try and get past it, got another crash. This time the backtrace showedfile was being optimized out, relevent section of it here:


Thread 1 (Thread 0x7ffff3586140 (LWP 84060) "caja"):
#0  0x00007ffff6ffc085 in g_type_check_instance_cast () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#1  0x00005555556b0fe4 in file_icons_changed (file=<optimized out>, widget=0x55555650f700) at caja-file-conflict-dialog.c:84
        surface = 0x555556f4b000
#2  0x00007ffff6fda4ef in g_closure_invoke () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#3  0x00007ffff6fec306 in  () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#4  0x00007ffff6ff2689 in g_signal_emit_valist () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#5  0x00007ffff6ff2bb2 in g_signal_emit () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#6  0x000055555565822a in caja_file_emit_changed (file=0x5555565fff40) at caja-file.c:7753
        link_files = <optimized out>
        p = <optimized out>
        __func__ = "caja_file_emit_changed"
cwendling commented 2 years ago

Still beats me… maybe you could try and run under Valgrind's memcheck to see if it could give us the initial source of invalid pointer? Simply valgrind caja [args] should do

lukefromdc commented 2 years ago

Got this result, again with that last patch applied:

==134730== Memcheck, a memory error detector
==134730== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==134730== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==134730== Command: caja [args]
==134730== 
Initializing caja-sendto extension
Initializing caja-xattr-tags extension
Initializing caja-share extension
Initializing caja-open-terminal extension
Initializing caja-image-converter extension
Initializing caja-wallpaper extension
==134730== Invalid read of size 8
==134730==    at 0x48F4C63: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x58A4893: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x58A4BF7: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x58A4C9E: g_main_context_iteration (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x56EE29C: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.7200.1)
==134730==    by 0x15A402: main (caja-main.c:271)
==134730==  Address 0x959c4a8 is 8 bytes inside a block of size 24 free'd
==134730==    at 0x484217B: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==134730==    by 0x58A01E9: g_list_delete_link (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x48F4C53: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x58A4893: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x58A4BF7: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x58A4C9E: g_main_context_iteration (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x56EE29C: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.7200.1)
==134730==    by 0x15A402: main (caja-main.c:271)
==134730==  Block was alloc'd at
==134730==    at 0x483F7B5: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==134730==    by 0x58AA8B8: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x58C2150: g_slice_alloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x589FCA5: g_list_prepend (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x48F47D5: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x48F481B: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x48F4AD3: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x48F5659: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x48F2D90: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x48F2E44: mate_bg_draw (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x48F31C9: mate_bg_create_surface_scale (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730==    by 0x48F2FC4: mate_bg_create_surface (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==134730== 
==134730== Invalid read of size 8
==134730==    at 0x4E43C8D: gtk_widget_get_scale_factor (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x27D434: file_icons_changed (caja-file-conflict-dialog.c:76)
==134730==    by 0x58084EE: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x581A305: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820688: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820BB1: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x217CD9: caja_file_emit_changed (caja-file.c:7753)
==134730==    by 0x1F9CEF: caja_directory_emit_change_signals (caja-directory.c:695)
==134730==    by 0x217EA4: caja_file_changed (caja-file.c:7700)
==134730==    by 0x1FA147: caja_directory_notify_files_added (caja-directory.c:923)
==134730==    by 0x1FED72: caja_file_changes_consume_changes (caja-file-changes-queue.c:345)
==134730==    by 0x1FF445: copy_job_done (caja-file-operations.c:4625)
==134730==  Address 0xb927780 is 336 bytes inside a block of size 392 free'd
==134730==    at 0x484217B: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==134730==    by 0x5828B85: g_type_free_instance (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x4BB378F: ??? (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x4C03A56: ??? (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x5808427: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x581A26F: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820688: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820BB1: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x4E48C13: ??? (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x580E748: g_object_run_dispose (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x4BB378F: ??? (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x4C03A56: ??? (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==  Block was alloc'd at
==134730==    at 0x483F7B5: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==134730==    by 0x58AA8B8: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x58C2150: g_slice_alloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x58C27B9: g_slice_alloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==134730==    by 0x582884D: g_type_create_instance (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x580D97C: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x580ED8C: g_object_new_with_properties (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x580F6B0: g_object_new (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x4CBF2DA: gtk_image_new_from_surface (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x27CD2A: file_list_ready_cb (caja-file-conflict-dialog.c:233)
==134730==    by 0x2163F2: file_list_file_ready_callback (caja-file.c:8321)
==134730==    by 0x2163F2: file_list_file_ready_callback (caja-file.c:8311)
==134730==    by 0x1F039D: ready_callback_call (caja-directory-async.c:1368)
==134730== 
==134730== Invalid read of size 8
==134730==    at 0x4E43C96: gtk_widget_get_scale_factor (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x27D434: file_icons_changed (caja-file-conflict-dialog.c:76)
==134730==    by 0x58084EE: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x581A305: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820688: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820BB1: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x217CD9: caja_file_emit_changed (caja-file.c:7753)
==134730==    by 0x1F9CEF: caja_directory_emit_change_signals (caja-directory.c:695)
==134730==    by 0x217EA4: caja_file_changed (caja-file.c:7700)
==134730==    by 0x1FA147: caja_directory_notify_files_added (caja-directory.c:923)
==134730==    by 0x1FED72: caja_file_changes_consume_changes (caja-file-changes-queue.c:345)
==134730==    by 0x1FF445: copy_job_done (caja-file-operations.c:4625)
==134730==  Address 0xaaaaaaaaaaaaaaaa is not stack'd, malloc'd or (recently) free'd
==134730== 
==134730== 
==134730== Process terminating with default action of signal 11 (SIGSEGV)
==134730==  General Protection Fault
==134730==    at 0x4E43C96: gtk_widget_get_scale_factor (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2404.30)
==134730==    by 0x27D434: file_icons_changed (caja-file-conflict-dialog.c:76)
==134730==    by 0x58084EE: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x581A305: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820688: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x5820BB1: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.7200.1)
==134730==    by 0x217CD9: caja_file_emit_changed (caja-file.c:7753)
==134730==    by 0x1F9CEF: caja_directory_emit_change_signals (caja-directory.c:695)
==134730==    by 0x217EA4: caja_file_changed (caja-file.c:7700)
==134730==    by 0x1FA147: caja_directory_notify_files_added (caja-directory.c:923)
==134730==    by 0x1FED72: caja_file_changes_consume_changes (caja-file-changes-queue.c:345)
==134730==    by 0x1FF445: copy_job_done (caja-file-operations.c:4625)
==134730== 
==134730== HEAP SUMMARY:
==134730==     in use at exit: 21,422,376 bytes in 180,573 blocks
==134730==   total heap usage: 1,923,914 allocs, 1,743,341 frees, 376,837,683 bytes allocated
==134730== 
==134730== LEAK SUMMARY:
==134730==    definitely lost: 45,136 bytes in 91 blocks
==134730==    indirectly lost: 144,601 bytes in 5,882 blocks
==134730==      possibly lost: 44,309 bytes in 696 blocks
==134730==    still reachable: 18,324,610 bytes in 155,706 blocks
==134730==                       of which reachable via heuristic:
==134730==                         newarray           : 48 bytes in 1 blocks
==134730==         suppressed: 0 bytes in 0 blocks
==134730== Rerun with --leak-check=full to see details of leaked memory
==134730== 
==134730== For lists of detected and suppressed errors, rerun with: -s
==134730== ERROR SUMMARY: 4 errors from 3 contexts (suppressed: 0 from 0)
Segmentation fault
cwendling commented 2 years ago

OK, it really looks like those widgets are destroyed and the callbacks still hit. Could you try using g_signal_connect_object() again on top of @rbuj's patchset? (that I assume you're using here? if that's not the case, nevermind)

Also, there's a pretty scary libmate-desktop issue as well showing up here, maybe you could try and build with debugging symbols to get more info?

lukefromdc commented 2 years ago

Getting out of my skill range here

cwendling commented 2 years ago

could you paste your current diff?

lukefromdc commented 2 years ago
diff --git a/libcaja-private/caja-file-conflict-dialog.c b/libcaja-private/caja-file-conflict-dialog.c
index 9d86d15c..233f47ae 100644
--- a/libcaja-private/caja-file-conflict-dialog.c
+++ b/libcaja-private/caja-file-conflict-dialog.c
@@ -68,27 +68,18 @@ G_DEFINE_TYPE_WITH_PRIVATE (CajaFileConflictDialog,
                             GTK_TYPE_DIALOG);

 static void
-file_icons_changed (CajaFile *file,
-                    CajaFileConflictDialog *fcd)
+file_icons_changed (CajaFile  *file,
+                    GtkWidget *widget)
 {
     cairo_surface_t *surface;

-    surface = caja_file_get_icon_surface (fcd->details->destination,
+    surface = caja_file_get_icon_surface (file,
                                           CAJA_ICON_SIZE_LARGE,
                                           FALSE,
-                                          gtk_widget_get_scale_factor (fcd->details->dest_image),
+                                          gtk_widget_get_scale_factor (widget),
                                           CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);

-    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->dest_image), surface);
-    cairo_surface_destroy (surface);
-
-    surface = caja_file_get_icon_surface (fcd->details->source,
-                                          CAJA_ICON_SIZE_LARGE,
-                                          FALSE,
-                                          gtk_widget_get_scale_factor (fcd->details->src_image),
-                                          CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);
-
-    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->src_image), surface);
+    gtk_image_set_from_surface (GTK_IMAGE (widget), surface);
     cairo_surface_destroy (surface);
 }

@@ -371,9 +362,11 @@ file_list_ready_cb (GList *files,
     caja_file_monitor_add (dest, fcd, CAJA_FILE_ATTRIBUTES_FOR_ICON);

     details->src_handler_id = g_signal_connect (src, "changed",
-                              G_CALLBACK (file_icons_changed), fcd);
+                                                G_CALLBACK (file_icons_changed),
+                                                fcd->details->src_image);
     details->dest_handler_id = g_signal_connect (dest, "changed",
-                               G_CALLBACK (file_icons_changed), fcd);
+                                                 G_CALLBACK (file_icons_changed),
+                                                 fcd->details->dest_image);
 }

 static void
cwendling commented 2 years ago

and what is the HEAD commit? (e.g. on what is this a diff)

lukefromdc commented 2 years ago

Current master, again you are getting past my skillset

lukefromdc commented 2 years ago

I just pulled a copy of the file out of my last build (from master as of today) and applied the patch

cwendling commented 2 years ago

git rev-parse HEAD

lukefromdc commented 2 years ago

https://github.com/mate-desktop/caja/pull/1632 also had no effect

cwendling commented 2 years ago

yeah #1632 is basically the diff you pasted…

this shouldn't really help (but might avoid the crash in a sad way thanks to the checks -- not a real solution), but let's see:

diff --git a/libcaja-private/caja-file-conflict-dialog.c b/libcaja-private/caja-file-conflict-dialog.c
index 9d86d15c..574c926a 100644
--- a/libcaja-private/caja-file-conflict-dialog.c
+++ b/libcaja-private/caja-file-conflict-dialog.c
@@ -68,27 +68,18 @@ G_DEFINE_TYPE_WITH_PRIVATE (CajaFileConflictDialog,
                             GTK_TYPE_DIALOG);

 static void
-file_icons_changed (CajaFile *file,
-                    CajaFileConflictDialog *fcd)
+file_icons_changed (CajaFile  *file,
+                    GtkWidget *widget)
 {
     cairo_surface_t *surface;

-    surface = caja_file_get_icon_surface (fcd->details->destination,
+    surface = caja_file_get_icon_surface (file,
                                           CAJA_ICON_SIZE_LARGE,
                                           FALSE,
-                                          gtk_widget_get_scale_factor (fcd->details->dest_image),
+                                          gtk_widget_get_scale_factor (widget),
                                           CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);

-    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->dest_image), surface);
-    cairo_surface_destroy (surface);
-
-    surface = caja_file_get_icon_surface (fcd->details->source,
-                                          CAJA_ICON_SIZE_LARGE,
-                                          FALSE,
-                                          gtk_widget_get_scale_factor (fcd->details->src_image),
-                                          CAJA_FILE_ICON_FLAGS_USE_THUMBNAILS);
-
-    gtk_image_set_from_surface (GTK_IMAGE (fcd->details->src_image), surface);
+    gtk_image_set_from_surface (GTK_IMAGE (widget), surface);
     cairo_surface_destroy (surface);
 }

@@ -370,10 +361,12 @@ file_list_ready_cb (GList *files,
     caja_file_monitor_add (src, fcd, CAJA_FILE_ATTRIBUTES_FOR_ICON);
     caja_file_monitor_add (dest, fcd, CAJA_FILE_ATTRIBUTES_FOR_ICON);

-    details->src_handler_id = g_signal_connect (src, "changed",
-                              G_CALLBACK (file_icons_changed), fcd);
-    details->dest_handler_id = g_signal_connect (dest, "changed",
-                               G_CALLBACK (file_icons_changed), fcd);
+    details->src_handler_id = g_signal_connect_object (src, "changed",
+                                                       G_CALLBACK (file_icons_changed),
+                                                       fcd->details->src_image, 0);
+    details->dest_handler_id = g_signal_connect_object (dest, "changed",
+                                                        G_CALLBACK (file_icons_changed),
+                                                        fcd->details->dest_image, 0);
 }

 static void
lukefromdc commented 2 years ago

That last patch did stop the crash in my tests

cwendling commented 2 years ago

wow, okay, interesting. Could you run under Valrgind to see if there's still a problem, just less serious?

lukefromdc commented 2 years ago

Got this under valgrind:

luke@ubuntu:~$ valgrind caja
==195183== Memcheck, a memory error detector
==195183== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==195183== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==195183== Command: caja
==195183== 
Initializing caja-sendto extension
Initializing caja-xattr-tags extension
Initializing caja-share extension
Initializing caja-open-terminal extension
Initializing caja-image-converter extension
Initializing caja-wallpaper extension
==195183== Invalid read of size 8
==195183==    at 0x48F4C63: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x58A4893: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x58A4BF7: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x58A4C9E: g_main_context_iteration (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x56EE29C: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.7200.1)
==195183==    by 0x15A402: main (caja-main.c:271)
==195183==  Address 0xb429388 is 8 bytes inside a block of size 24 free'd
==195183==    at 0x484217B: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==195183==    by 0x58A01E9: g_list_delete_link (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x48F4C53: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x58A4893: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x58A4BF7: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x58A4C9E: g_main_context_iteration (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x56EE29C: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.7200.1)
==195183==    by 0x15A402: main (caja-main.c:271)
==195183==  Block was alloc'd at
==195183==    at 0x483F7B5: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==195183==    by 0x58AA8B8: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x58C2150: g_slice_alloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x589FCA5: g_list_prepend (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.1)
==195183==    by 0x48F47D5: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x48F481B: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x48F4AD3: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x48F5659: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x48F2D90: ??? (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x48F2E44: mate_bg_draw (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x48F31C9: mate_bg_create_surface_scale (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183==    by 0x48F2FC4: mate_bg_create_surface (in /usr/lib/x86_64-linux-gnu/libmate-desktop-2.so.17.1.4)
==195183== 

(caja:195183): Gdk-CRITICAL **: 13:52:03.699: gdk_window_thaw_toplevel_updates: assertion 'window->update_and_descendants_freeze_count > 0' failed
cwendling commented 2 years ago

OK, so that looks good. Weird you don't get things like (caja:1234): GLib-GObject-WARNING **: 12:34:56.789: ../../../gobject/gsignal.c:2731: instance '0xaeb5a50' has no handler with id '2345' as my patch should remove a couple more lines though, and that might be the source of the initial problems (as it suggests the manual disconnection of the handlers doesn't happen as it should). Anyway, if it solves the issue I'll come up with a proper patch to review as the idea is sound, maybe tomorrow.

rbuj commented 2 years ago

I closed #1632 so I don't want to lock the fix

cwendling commented 2 years ago

@lukefromdc Could you try the diff below and tell me what happens? I'd really like to actually understand what's going on, although I'll end up (also?) submitting a version of the above that worked for you.

diff --git a/libcaja-private/caja-file-conflict-dialog.c b/libcaja-private/caja-file-conflict-dialog.c
index 9d86d15c..4228a900 100644
--- a/libcaja-private/caja-file-conflict-dialog.c
+++ b/libcaja-private/caja-file-conflict-dialog.c
@@ -679,33 +679,45 @@ caja_file_conflict_dialog_init (CajaFileConflictDialog *fcd)
 }

 static void
-do_finalize (GObject *self)
+do_dispose (GObject *self)
 {
     CajaFileConflictDialogPrivate *details =
         CAJA_FILE_CONFLICT_DIALOG (self)->details;

-    g_free (details->conflict_name);
-
     if (details->handle != NULL)
     {
         caja_file_list_cancel_call_when_ready (details->handle);
+        details->handle = NULL;
     }

     if (details->src_handler_id)
     {
         g_signal_handler_disconnect (details->source, details->src_handler_id);
         caja_file_monitor_remove (details->source, self);
+        details->src_handler_id = 0;
     }

     if (details->dest_handler_id)
     {
         g_signal_handler_disconnect (details->destination, details->dest_handler_id);
         caja_file_monitor_remove (details->destination, self);
+        details->dest_handler_id = 0;
     }

-    caja_file_unref (details->source);
-    caja_file_unref (details->destination);
-    caja_file_unref (details->dest_dir);
+    g_clear_pointer (&details->source, caja_file_unref);
+    g_clear_pointer (&details->destination, caja_file_unref);
+    g_clear_pointer (&details->dest_dir, caja_file_unref);
+
+    G_OBJECT_CLASS (caja_file_conflict_dialog_parent_class)->dispose (self);
+}
+
+static void
+do_finalize (GObject *self)
+{
+    CajaFileConflictDialogPrivate *details =
+        CAJA_FILE_CONFLICT_DIALOG (self)->details;
+
+    g_free (details->conflict_name);

     G_OBJECT_CLASS (caja_file_conflict_dialog_parent_class)->finalize (self);
 }
@@ -713,6 +725,7 @@ do_finalize (GObject *self)
 static void
 caja_file_conflict_dialog_class_init (CajaFileConflictDialogClass *klass)
 {
+    G_OBJECT_CLASS (klass)->dispose = do_dispose;
     G_OBJECT_CLASS (klass)->finalize = do_finalize;
 }
lukefromdc commented 2 years ago

This stopped the crash, didn't get any known file conflict dialog issues running this

On 6/2/2022 at 9:29 AM, "Colomban Wendling" @.***> wrote:

@.*** Could you try the diff below and tell me what happens? I'd really like to actually understand what's going on, although I'll end up (also?) submitting a version of the above that worked for you.

diff --git a/libcaja-private/caja-file-conflict-dialog.c b/libcaja-
private/caja-file-conflict-dialog.c
index 9d86d15c..4228a900 100644
--- a/libcaja-private/caja-file-conflict-dialog.c
+++ b/libcaja-private/caja-file-conflict-dialog.c
@@ -679,33 +679,45 @@ caja_file_conflict_dialog_init 
(CajaFileConflictDialog *fcd)
}

static void
-do_finalize (GObject *self)
+do_dispose (GObject *self)
{
    CajaFileConflictDialogPrivate *details =
        CAJA_FILE_CONFLICT_DIALOG (self)->details;

-    g_free (details->conflict_name);
-
    if (details->handle != NULL)
    {
        caja_file_list_cancel_call_when_ready (details->handle);
+        details->handle = NULL;
    }

    if (details->src_handler_id)
    {
        g_signal_handler_disconnect (details->source, details-
>src_handler_id);
        caja_file_monitor_remove (details->source, self);
+        details->src_handler_id = 0;
    }

    if (details->dest_handler_id)
    {
        g_signal_handler_disconnect (details->destination, 
details->dest_handler_id);
        caja_file_monitor_remove (details->destination, self);
+        details->dest_handler_id = 0;
    }

-    caja_file_unref (details->source);
-    caja_file_unref (details->destination);
-    caja_file_unref (details->dest_dir);
+    g_clear_pointer (&details->source, caja_file_unref);
+    g_clear_pointer (&details->destination, caja_file_unref);
+    g_clear_pointer (&details->dest_dir, caja_file_unref);
+
+    G_OBJECT_CLASS (caja_file_conflict_dialog_parent_class)-
>dispose (self);
+}
+
+static void
+do_finalize (GObject *self)
+{
+    CajaFileConflictDialogPrivate *details =
+        CAJA_FILE_CONFLICT_DIALOG (self)->details;
+
+    g_free (details->conflict_name);

    G_OBJECT_CLASS (caja_file_conflict_dialog_parent_class)-
>finalize (self);
}
@@ -713,6 +725,7 @@ do_finalize (GObject *self)
static void
caja_file_conflict_dialog_class_init (CajaFileConflictDialogClass 
*klass)
{
+    G_OBJECT_CLASS (klass)->dispose = do_dispose;
    G_OBJECT_CLASS (klass)->finalize = do_finalize;
}

-- Reply to this email directly or view it on GitHub: https://github.com/mate-desktop/caja/issues/1630#issuecomment- 1144648715 You are receiving this because you were mentioned.

Message ID: @.***>

raveit65 commented 2 years ago

You should cherry-pick/packport fixes to 1.26 branch, in result i can make a new release,

lukefromdc commented 2 years ago

Cherrypicked all three commit after local test build and run went fine. All three commits are shown by git log and in https://github.com/mate-desktop/caja/network but for some reason the site page here only shows two of them

lukefromdc commented 1 year ago

Also note that on a LONG file replacement jog such as a bunch of video files, the crash may occur partially through or even near the end of the file transfer job, the progress bar appears normally as file conflict dialog disappears. The backtrace showed the file conflict dialog as the problem though

cwendling commented 1 year ago

@lukefromdc do you mean the issue is actually not resolved? If so, would you have a backtrace of the case you mention, e.g. long after the confirmation dialog closed? Or any backtrace on top of those fixes for that matter.

lukefromdc commented 1 year ago

No, this was before the issue was fixed. I haven't had that crash once since the fix was merged

lukefromdc commented 1 year ago

Also note that may previous comment here that was posted long ago seem to have just shown up today

cwendling commented 1 year ago

ah OK, weird, but cool then :)