c-koi / gmic-qt

G'MIC-Qt is a versatile front-end to the image processing framework G'MIC.
GNU General Public License v3.0
207 stars 54 forks source link

Consistenly crashing on MacOS 10.14 whether hosted in Krita or Gimp #160

Open ShirtlessWebmaster opened 2 years ago

ShirtlessWebmaster commented 2 years ago

I am big fan of the G'Mic filters and they have become an integral part of my idea generation process. A big thanks to anyone contributing to this brilliant and inspiring tool.

I am struggling to find a way of hosting the GUI of Gmic on MacOS. In both Krita or GIMP the plug-in consistently crashes after a few moves. Somebody over on Reddit suggested I posted my crash logs to here, so here goes. Let me know if there's anything else I can do to provide useful information.

krita gmic crashlog macos 10-14 (mojave).txt

ShirtlessWebmaster commented 2 years ago

And here's a crashlog from Krita crashing on my laptop also running MacOS 10.14 krita macos (10.14) crashlog laptop.txt .

dtschump commented 2 years ago

Thanks for your report. We don't have a Mac, so we'll have a hard time being useful here. Anyway, I remember that for years, we got frequent G'MIC crashes in the past on MacOS, with frequent reports by @karo11 . After years of investigation, we found out it was due to the fact that on MacOS, the default stack size allocated for a new thread is ridiculously low (512 KB by default), and that it was not enough for the G'MIC commands running with multi-threading. Your log suggests it could be related (I see multiple threads trying to run there).

That particularly why we have this hack in file gmic.cpp: https://github.com/GreycLab/gmic/blob/master/src/gmic.cpp#L10765 where we try forcing the stack size of a new thread to be 8 Mo (which is actually the default size for Linux systems, where everything runs fine :) ). But that hack is done for the moment only for threads that are created by the G'MIC interpreter itself, not threads that could be created by the G'MIC-Qt plug-in or by Krita. I think G'MIC-Qt also creates threads (@c-koi may confirm), so it may be necessary to apply the same kind of hack for those threads too. This also applies for possible threads created by Krita (@amyspark may confirm).

That's my take on the subject.

dtschump commented 2 years ago

On MacOSX, the default stack size for the main thread is actually 8Mo while the stack size of a newly created thread is 512KB. My guess is that if the G'MIC interpreter is instanciated in a thread that is not the main thread, then it has to work with 512KB of stack size, which is (obviously :) ) not enough, so it will crash. I think this is the caller's responsibility (G'MIC-Qt or Krita) to ensure enough stack size to the new thread for the task to be done correctly. I'm not sure it's even possible to change the stack size of a thread when the thread has started.

amyspark commented 2 years ago

As for Krita, we already run a 16MB stack for G'MIC and Krita itself-- see https://github.com/amyspark/gmic/blob/8a6ce71ddc868400eb25676f51c43bd014e0e33d/gmic-qt/CMakeLists.txt#L255-L267, so that can be safely excluded.

ShirtlessWebmaster commented 2 years ago

It's lovely to hear some background information and knowing the issue has been researched in the past. If there's anything I could try regarding the gmic.cpp-hack I'd be able to do it with specific instructions on which files to alter on my system.

dtschump commented 2 years ago

As for Krita, we already run a 16MB stack for G'MIC and Krita itself-- see https://github.com/amyspark/gmic/blob/8a6ce71ddc868400eb25676f51c43bd014e0e33d/gmic-qt/CMakeLists.txt#L255-L267, so that can be safely excluded.

But that is only for Windows, not for MacOS, right?

amyspark commented 2 years ago

But that is only for Windows, not for MacOS, right?

facepalms Oh my, yes. I'll fix it on our end.

amyspark commented 2 years ago

I've had some time to review this on our end. macOS is indeed not covered by our workaround, but that wouldn't have affected the worker threads as detailed in 1. G'MIC already works around this in https://github.com/GreycLab/gmic/blob/master/src/gmic.cpp#L10779.

Now that I had a closer look at that call stack, I've had one such report before which I couldn't reproduce. What filter were you running when the crash happened, and what was the image size?

dtschump commented 2 years ago

G'MIC already works around this in https://github.com/GreycLab/gmic/blob/master/src/gmic.cpp#L10779.

Except that if your Krita-specific code launches the G'MIC interpreter in its own thread ? I don't know if this is the case or not. (And thanks for your postcard, I've received it today :) ).

amyspark commented 2 years ago

Except that if your Krita-specific code launches the G'MIC interpreter in its own thread ?

As with every plugin, yes. But the crashing thread is a separate worker thread, which falls under your code's purview.

karo11 commented 2 years ago

I am actually not with my mac.

However let me know how to try. I am actually using gmic and gmic-gimp from mac ports and git develop builds on my macbook.

David Tschumperlé @.***> schrieb am Mi., 10. Aug. 2022, 15:38:

On MacOSX, the default stack size for the main thread is actually 8Mo while the stack size of a newly created thread is 512KB. My guess is that if the G'MIC interpreter is instanciated in a thread that is not the main thread, then it has to work with 512KB of stack size, which is (obviously :) ) not enough, so it will crash. I think this is the caller's responsibility (G'MIC-Qt or Krita) to ensure enough stack size to the new thread for the task to be done correctly. I'm not sure it's even possible to change the stack size of a thread when the thread has started.

— Reply to this email directly, view it on GitHub https://github.com/c-koi/gmic-qt/issues/160#issuecomment-1210687475, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJJIV2ZB5WYNJIQPU4JQ73VYOWGLANCNFSM56ELGVHA . You are receiving this because you were mentioned.Message ID: @.***>

cgilles commented 1 year ago

Crash is also reproducible under digiKam with MacOS. UPSTREAM BUGS:

https://bugs.kde.org/show_bug.cgi?id=422996 https://bugs.kde.org/show_bug.cgi?id=467215

dtschump commented 1 year ago

This may be related to https://github.com/GreycLab/gmic/issues/437 If I remember well, X11 display support is not activated on MacOSX, and we recently found a bug that prevented mutexes to work properly if display is disabled. The fix is quite simple so it may be worth trying this.

cgilles commented 1 year ago

Hi all ,

I updated gmic/gmicqt code in digiKam plugin, increase the stack size under MacOS, and now crash only appear rarely.

https://i.imgur.com/xJx5RuO.png

It's clear that a stack overflow is the problem. The MacOS is a painful, i already fixed plenty of internal code in digiKam to allocate with new operator instead local allocation. A same fix must be done in Gmic code everywhere.

It's still a crash due to reopen the digiKam GmicQt plugin more than one time in the same instance of digiKam. I think about a missing delete somewhere...

@amyspark: you need to apply the same rules in Krita to fix the crash about stack overflow :

https://invent.kde.org/graphics/digikam/-/blob/master/core/cmake/rules/RulesMacOS.cmake

It's must be done with krita compilation, not your plugin...

Best

Gilles Caulier

cgilles commented 1 year ago

Note: gmicqt is compiled with clang under MacOS, not G++. Macports is used to compile all the dependencies (in fact the whole digiKam package in fact)...

Gilles Caulier