Bug 217946 - x11-fm/thunar dumps core on file or dir rename
Summary: x11-fm/thunar dumps core on file or dir rename
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Many People
Assignee: Guido Falsi
URL: https://bugzilla.xfce.org/show_bug.cg...
Keywords:
Depends on: 199872
Blocks:
  Show dependency treegraph
 
Reported: 2017-03-20 12:00 UTC by Marko Cupać
Modified: 2018-02-22 12:02 UTC (History)
8 users (show)

See Also:
madpilot: maintainer-feedback+
madpilot: merge-quarterly?


Attachments
Backtrace with issue reproduced in pcmanfm-qt (1.77 KB, text/plain)
2017-10-29 00:08 UTC, Kevin Zheng
no flags Details
Backtrace with different issue in thunar (3.48 KB, text/plain)
2017-11-01 17:40 UTC, Kevin Zheng
no flags Details
xfdesktop crash backtrace (2.73 KB, text/plain)
2017-11-13 17:06 UTC, Guido Falsi
no flags Details
Thunar crash workaround (1.89 KB, patch)
2018-02-14 15:49 UTC, Guido Falsi
no flags Details | Diff
Thunar crash workaround (1.99 KB, patch)
2018-02-16 21:15 UTC, Guido Falsi
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marko Cupać 2017-03-20 12:00:17 UTC
Hi,

problem with Thunar randomly dumping core on file/folder rename persists, although it was marked as solved here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208341

I am currently on 11.0-RELEASE-p8 (x64) and Thunar-1.6.11 which I build myself in poudriere.

I'd like to stress this is not something that happens once in a blue day - Thunar is more likely to crash on file/folder rename than not. I experience it on both UFS and ZFS file systems. I am not 100% sure, but I would say crashes are absent or at least not so frequent when renaming files and folders accessed via sftp:// in Thunar.

I'd be glad to help with testing patches.
Comment 1 Jochen Neumeister freebsd_committer 2017-03-21 18:22:09 UTC
(In reply to Marko Cupać from comment #0)

Hello,
I would like to understand the problem.
Which FreeBSD are you using? Do you have more information?
If you start Thunar from the console, which error comes with a dumping core exactly?
Comment 2 Marko Cupać 2017-03-23 13:13:19 UTC
Hi,

thank you for looking into it.

First, I must correct myself regarding reason and frequency of crashes, as over last few days of testing Thunar-1.6.11 I had no random crashes on rename from within Thunar when renames are performed within single Thunar window with single tab.

Crashes are 100% reproducible when there are two Thunar windows open, and I rename a folder in one window which is pwd in another window. This also happens when another application (I tested with easytag) renames folder which is pwd in Thunar. Multiple tabs within single Thunar window do not crash in this scenario (renaming directory in one tab which is pwd in another tab).

Above described crash throws this to console:
**
GLib-GIO:ERROR:glocalfilemonitor.c:433:g_file_monitor_source_handle_event: code should not be reached
Abort (core dumped)

I also noticed that Thunar, when invoked from console with `thunar', was started in daemon mode once. Consequent invokes started it in standard mode. What influences this?
Comment 3 Jochen Neumeister freebsd_committer 2017-03-30 09:40:03 UTC
I can reproduce this problem. Other peoples have the same Problem, when rename a directory. I think, i reported this Problem directly at xfce
Comment 4 Ben Woods freebsd_committer 2017-04-03 00:45:37 UTC
I have also experienced this problem.
Comment 5 Jochen Neumeister freebsd_committer 2017-04-03 00:58:57 UTC
(In reply to Ben Woods from comment #4)
https://bugzilla.xfce.org/show_bug.cgi?id=13468
Comment 6 Jochen Neumeister freebsd_committer 2017-04-03 01:01:47 UTC
(In reply to Ben Woods from comment #4)
can you set "Status" open?
Comment 7 Olivier Duchateau 2017-07-01 05:08:07 UTC
An update is available bug #220407, which contains the patch mentioned in comment #5
Comment 8 Olivier Duchateau 2017-07-01 05:09:33 UTC
(In reply to duchateau.olivier from comment #7)

With other improvements.
Comment 9 Guido Falsi freebsd_committer 2017-09-30 14:29:44 UTC
Adding bug 221679 as see also, since I'm almost sure this is the same issue.

I'm posting my findings on that bug report.
Comment 10 Kevin Zheng 2017-10-29 00:08:00 UTC
Created attachment 187552 [details]
Backtrace with issue reproduced in pcmanfm-qt

I've also managed to reproduce the same issue in pcmanfm-qt (which uses GLib). The backtraces on the Thunar crashes seem to show the same locking issues in the kqueue part.
Comment 11 Guido Falsi freebsd_committer 2017-10-29 07:02:20 UTC
(In reply to Kevin Zheng from comment #10)
> Created attachment 187552 [details]
> Backtrace with issue reproduced in pcmanfm-qt
> 
> I've also managed to reproduce the same issue in pcmanfm-qt (which uses
> GLib). The backtraces on the Thunar crashes seem to show the same locking
> issues in the kqueue part.

Can you also test the patch I posted on bug 199872 (for glib) and report back there if it works correctly for you?

Thanks!
Comment 12 Kevin Zheng 2017-11-01 03:24:16 UTC
(In reply to Guido Falsi from comment #11)
I'm running with your patch and haven't observed the crash. Since I wasn't able to reproduce it reliably before I can't be confident that the issue has been fixed. I'll report back if I can get the crash with the patch.
Comment 13 Kevin Zheng 2017-11-01 17:40:33 UTC
Created attachment 187655 [details]
Backtrace with different issue in thunar

I have this backtrace from Thunar. The crash doesn't seem to do with kqueue but still occurs in the file monitor.
Comment 14 Guido Falsi freebsd_committer 2017-11-01 20:41:51 UTC
(In reply to Kevin Zheng from comment #13)
> Created attachment 187655 [details]
> Backtrace with different issue in thunar
> 
> I have this backtrace from Thunar. The crash doesn't seem to do with kqueue
> but still occurs in the file monitor.

Were you renaming a file?

It's happened to me a few times, but I've been unable to reproduce it with debugging symbols to get a backtrace.

I can't promise anything, since understanding this kind of backtrace is very difficult, but I'll have a look.
Comment 15 Guido Falsi freebsd_committer 2017-11-13 17:06:07 UTC
Created attachment 187972 [details]
xfdesktop crash backtrace

I've hit another crash with xfdesktop which I am able to reproduce.

Here is a backtrace obtained from a Virtual Machine with debug binaries.

I'm still investigating this one too. I attach it here for further reference and in case someone else wants to look at it too.
Comment 16 Kevin Zheng 2017-11-13 18:54:39 UTC
(In reply to Guido Falsi from comment #15)
I've had a xfdesktop crash due to glib but I haven't been able to reproduce it. What are the steps to reproducing the crash?
Comment 17 Guido Falsi freebsd_committer 2017-11-13 19:42:41 UTC
(In reply to Kevin Zheng from comment #16)
> (In reply to Guido Falsi from comment #15)
> I've had a xfdesktop crash due to glib but I haven't been able to reproduce
> it. What are the steps to reproducing the crash?

It happens when renaming directories on the desktop, but I was unable to reliably reproduce it.

I was saving a book on the desktop using calibre. To do that I opened the save book requester of that app and created a directory, then renamed it inside the requester and there goes the crash in a reliable way.

I'm almost sure it can be triggered in any requester, but just in case it depends on which one, calibre uses py-qt5, so I'd try with some qt5 app and eventually with calibre itself.
Comment 18 Guido Falsi freebsd_committer 2017-11-13 21:23:24 UTC
Regarding the xfdesktop crash:

xfce is using the (deprecated) G_FILE_MONITOR_SEND_MOVED, which causes the glib backend to generate G_FILE_MONITOR_EVENT_MOVED events, but these are unsupported in the GIO backend. There's an assertion which fails causing the crash.

I'm unsure how to attack this.

One option is to disable the G_FILE_MONITOR_SEND_MOVED flag and let xfdesktop cope with separate deleted and created events.

Another option could be to modify xfdesktop to use the non deprecated and apparently supported G_FILE_MONITOR_WATCH_MOVES flag (and related events). This is definitely more difficult.

I have seen no changes to this code in the upstream repositories.
Comment 19 Kevin Zheng 2017-11-13 21:46:51 UTC
(In reply to Guido Falsi from comment #18)
I think the former (disable G_FILE_MONITOR_SEND_MOVED) makes more sense until upstream switches to a function that is not deprecated. It's probably fine to deal with an add followed by a remove rather than xfdesktop crashes.
Comment 20 Guido Falsi freebsd_committer 2017-11-13 22:20:35 UTC
(In reply to Kevin Zheng from comment #19)
> (In reply to Guido Falsi from comment #18)
> I think the former (disable G_FILE_MONITOR_SEND_MOVED) makes more sense
> until upstream switches to a function that is not deprecated. It's probably
> fine to deal with an add followed by a remove rather than xfdesktop crashes.

Actually, The problem is in glib kqueue backend, which is generating the deprecated event. I think I have the correct fix for that, I'm going to revisit the patch I attached to bug 199872 to fix this issue too...At least, that's if testing gives good results.
Comment 21 Guido Falsi freebsd_committer 2017-11-13 22:24:26 UTC
(In reply to Kevin Zheng from comment #13)
> Created attachment 187655 [details]
> Backtrace with different issue in thunar
> 
> I have this backtrace from Thunar. The crash doesn't seem to do with kqueue
> but still occurs in the file monitor.

I had a look at the backtrace. Unluckily it's missing data for stack positions 1 and 2, so we don't know exactly what's happening there.

Anyway, it fails in a compare function, which is called via a pointer, so I don't know which one that is. Some other code from some consumer set that callback, that information can be extracted only at runtime with a debugger.

I'm almost sure the crash is caused by that compare function.

I have been unable to reproduce this crash, I'm using en_US locale, maybe you're using a different one? That could cause string comparison errors, if the code has some defects.

Otherwise, could you recompile some more parts with debugging symbols and try again? maybe we can catch the missing calls.
Comment 22 Guido Falsi freebsd_committer 2017-11-16 23:15:25 UTC
(In reply to Guido Falsi from comment #20)
> (In reply to Kevin Zheng from comment #19)
> > (In reply to Guido Falsi from comment #18)
> > I think the former (disable G_FILE_MONITOR_SEND_MOVED) makes more sense
> > until upstream switches to a function that is not deprecated. It's probably
> > fine to deal with an add followed by a remove rather than xfdesktop crashes.
> 
> Actually, The problem is in glib kqueue backend, which is generating the
> deprecated event. I think I have the correct fix for that, I'm going to
> revisit the patch I attached to bug 199872 to fix this issue too...At least,
> that's if testing gives good results.

I added a revised patch addressing this issue in bug #199872.

Please test that and report back. It works for me and I don't see any problem with it, but it does need testing to make sure.
Comment 23 Kevin Zheng 2017-11-18 20:04:36 UTC
(In reply to Guido Falsi from comment #22)
I can't tell what's going on, but it feels like whenever I do an operation that would have caused a crash, Thunar just exits without leaving a core dump.
Comment 24 Guido Falsi freebsd_committer 2017-11-18 20:41:43 UTC
(In reply to Kevin Zheng from comment #23)
> (In reply to Guido Falsi from comment #22)
> I can't tell what's going on, but it feels like whenever I do an operation
> that would have caused a crash, Thunar just exits without leaving a core
> dump.

The last patch I added is the same as the one before, but with a small extra change aimed at fixing the crash in xfdesktop. In fact it should not change much in thunar.

There still is a crash in thunar I'm trying to reproduce with debugging packages. There is a chance this last crash which I still have to address is unrelated to glib, but I cannot be sure until I get a backtrace.

I'm not sure what you're seeing, Have you tried running thunar via gdb directly?
Comment 25 vali gholami 2017-11-26 20:50:05 UTC
MARKED AS SPAM
Comment 26 commit-hook freebsd_committer 2018-01-28 20:29:59 UTC
A commit references this bug:

Author: kwm
Date: Sun Jan 28 20:29:16 UTC 2018
New revision: 460230
URL: https://svnweb.freebsd.org/changeset/ports/460230

Log:
  Fix another crash bug in the kqueue backend.

  PR:		199872 217946

Changes:
  head/devel/glib20/Makefile
  head/devel/glib20/files/patch-gio_kqueue_kqueue-helper.c
Comment 27 commit-hook freebsd_committer 2018-01-30 07:04:42 UTC
A commit references this bug:

Author: kwm
Date: Tue Jan 30 07:04:22 UTC 2018
New revision: 460371
URL: https://svnweb.freebsd.org/changeset/ports/460371

Log:
  MFH: r460052 r460230

  Update glib to 2.50.3.

  Also redo the kqueue patches. Now we patch files only once, and add some
  bits that got lost somewhere (which is probably my fault). Which where
  causing crashes when for example nautilus or thundar where monitoring
  directories and files where added/removed.

  PR:		199872

  Fix another crash bug in the kqueue backend.

  PR:		199872 217946

  Approved by:	ports-secteam (swills@)

Changes:
_U  branches/2018Q1/
  branches/2018Q1/devel/glib20/Makefile
  branches/2018Q1/devel/glib20/distinfo
  branches/2018Q1/devel/glib20/files/patch-bug739424
  branches/2018Q1/devel/glib20/files/patch-bug778515
  branches/2018Q1/devel/glib20/files/patch-gio_kqueue_gkqueuefilemonitor.c
  branches/2018Q1/devel/glib20/files/patch-gio_kqueue_kqueue-helper.c
Comment 28 Guido Falsi freebsd_committer 2018-02-14 15:49:59 UTC
Created attachment 190622 [details]
Thunar crash workaround

I created a patch for the thunar port based on what I found in the xfce bug report I added in the "see also" field.

This last crash in thunar is not caused by glib, but is a thunar internal race condition in sorting and rereading files when a file is deleted in a window.

My patch is not a proper fix. Just a workaround I hacked and which seems to avoid the crash.

Some quick testing seems to prove the patch actually prevents it from crashing on my machine. I'm now running with this patch, and will try to make it crash again.

I'd really like to have someone else test this too before committing this to the official tree.

Can someone test thunar port with this patch and report back?
Comment 29 Guido Falsi freebsd_committer 2018-02-14 15:53:31 UTC
BTW the patch does two things:

First I added back a stanza which used to be in the official tree, but was removed at one point(was it a mistake?). in the function thunar_file_reload() It checks for the file variable to actually point to a file and makes the function return early if it does not.

The second part replaces two calls to strcmp() (these are the ones in which the crash was happening) with the glib provided g_strcmp0().

Looks like in certain situations a null pointer is passed to strcmp(), but g_strcmp0() is null safe and will not crash.
Comment 30 Guido Falsi freebsd_committer 2018-02-16 21:15:15 UTC
Created attachment 190703 [details]
Thunar crash workaround

Updated patch which applies after r462064
Comment 31 commit-hook freebsd_committer 2018-02-22 11:56:53 UTC
A commit references this bug:

Author: madpilot
Date: Thu Feb 22 11:56:45 UTC 2018
New revision: 462584
URL: https://svnweb.freebsd.org/changeset/ports/462584

Log:
  Add patches to thunar from upstream bug report to mitigate crash
  when renaming files.

  The patch replaces some calls to strcmp() which are sometimes getting
  NULL pointers, causing a crash, with safe calls to g_strcmp0()
  calls, which handle NULL pointers gracefully.

  I'm also adding a patch in another code path checking for a pointer
  to actually point to the correct structure and not being NULL.

  These patches seem to actually prevent the reported crash from
  happening.

  PR:		217946
  Submitted by:	Marko Cupac <marko.cupac@mimar.rs>
  Obtained from:	https://bugzilla.xfce.org/show_bug.cgi?id=12264

Changes:
  head/x11-fm/thunar/Makefile
  head/x11-fm/thunar/files/patch-thunar_thunar-file.c
Comment 32 Guido Falsi freebsd_committer 2018-02-22 12:02:50 UTC
I've committed the patch I filed here a week ago.

Everything seems to work fine for me, anyway I will be listening to any report for problems if they should arise.