Martchus / syncthingtray

Tray application and Dolphin/Plasma integration for Syncthing
https://martchus.github.io/syncthingtray/
Other
1.68k stars 44 forks source link

1.1.7 silently crashes on start on Windows #95

Closed sledgehammer999 closed 3 years ago

sledgehammer999 commented 3 years ago

Relevant components

Environment and versions

Bug description synthingtray silently exits upon launch. I don't get any error window. However, it seems to start syncthing in the background before exiting. Last known working version is 1.1.6 (Qt5 edition). I looked at the git logs between the 2 versions but I didn't find something obvious to me. I don't get any output in the console showing the problem. Moreover, on Windows I use a firewall that asks for allowing connections of new applications: The crash happens somewhere between launching syncthing in the background and starting to listen to 127.0.0.1. It never gets to listen to 127.0.0.1 because I don't get a firewall notification for this, while I get it for eg 1.1.6 (which continues to work).

Martchus commented 3 years ago

However, it seems to start syncthing in the background before exiting.

So I assume you configured Syncthing Tray to launch Syncthing then. This means the crash is happening after that.

I looked at the git logs between the 2 versions but I didn't find something obvious to me.

I'm also not aware of any changes which could lead to that. However, the changes within all dependencies would be relevant and I suppose Qt 5 received a few patches since then and I also updated GCC/mingw-w64 since then.


I have only tested the Qt 6 version under the latest Windows 10 and it worked. Does the Qt 6 version work for you? Did you use the i686 or the x86_64 build? Does it work with a prestine configuration file where the launcher is not configured yet (e.g. rename your config file temporarily for testing)?

How do you launch it? (There have been some changes to the console handling on Windows.)

sledgehammer999 commented 3 years ago

I suppose Qt 5 received a few patches

Semi-relevant: I am the maintainer of qbittorrent and that means I regularly do Qt5 builds on Windows. For me, Qt 5.15.2 works for qbittorrent on the same OS (my main OS).

Does the Qt 6 version work for you?

Even though Qt6 officially has dropped Win7 support I was curious. And the qt6 version works!!!

Now I tested the 32bit Qt5 version and it works too! So maybe a miscompilation of the 64bit qt5 version?

Does it work with a prestine configuration file where the launcher is not configured yet (e.g. rename your config file temporarily for testing)?

No it doesn't. Irrelevant: You shouldn't place the syncthingtray.ini directly in C:\Users\username\AppData\Roaming. It should be placed under its own subfolder IMO.

How do you launch it?

Either double-clicking or launching it via terminal without any arguments.

sledgehammer999 commented 3 years ago

I regularly do Qt5 builds on Windows

To be precise: I use the vanilla Qt 5.15.2 source and I manually build it on Windows. If you do cross-compiles with patches added on top of the vanilla source, then maybe one of those patches causes my problem. Even though it is weird that the 32bit version doesn't exhibit the issue.

Martchus commented 3 years ago

So maybe a miscompilation of the 64bit qt5 version?

Maybe, or a bug which only happens under certain conditions and they're only met on that build.

I am the maintainer of qbittorrent and that means I regularly do Qt5 builds on Windows.

Maybe you can reproduce the problem in a debug environment then or compare with a version built against your own Qt 5 libraries.

If you do cross-compiles with patches added on top of the vanilla source, then maybe one of those patches causes my problem.

Maybe. The patches I usually apply (mainly to make static linking work) haven't changed between the versions but maybe some of the patches picked-up from KDE's fork might cause this.

Martchus commented 3 years ago

Irrelevant: You shouldn't place the syncthingtray.ini directly in C:\Users\username\AppData\Roaming. It should be placed under its own subfolder IMO.

Maybe I should change that although one file vs. one folder shouldn't make a difference and the change wouldn't be compatible which can be annoying when testing different versions (like for the sake of investigating this issue).

Martchus commented 3 years ago

For the record, here the diff of the build env between 1.1.6 and 1.1.7: BUILDINFO-diff.txt

Martchus commented 3 years ago

I can reproduce the problem as well under Windows 10. The Qt 5 build works but the Qt 6 build doesn't; i686 works but x86_64 doesn't. However, I've just wanted to test a Qt 6 based development build (with a few more changes on top) and it crashed as well. Annoyingly one doesn't even get the usual error message from Windows, the app just exists quite quickly again.

I've just tested the Qt 5 builds of my other applications which I've released on the same day and which therefore used the identical Qt 5 build environment. They don't crash. I suppose it is a problem within Syncthing Tray after all or related to some Qt features or other dependencies only used by Syncthing Tray.

Martchus commented 3 years ago

Looks like it crashes when rendering svg in renderSvgImage() at some point. When calling the function with the same args at the very beginning it works. Maybe something before has already corrupted the program's state. On the other hand, when getting rid of the svg rendering just for FontAwesomeIcons than everything works. It doesn't help to downgrade to the Qt Svg version from the previous build and it looks like my new Qt 6 builds are affected as well.

sledgehammer999 commented 3 years ago

Sorry for not following up on this. I had a main disk failure and it took me time to migrate my stuff/settings to a new one. In any case, I am glad that you can replicate the crash. Unfortunately I am not so inclined to personally start digging into this further. For the moment, the qt6 build seems to work for me. I'll keep an eye on this issue and it's progress.

sledgehammer999 commented 3 years ago

Just a thought: Are you able to reproduce with 1.1.6 and the new qt5/qt6 libs? If no, then you should try doing a git bisect to pinpoint your bad commit.

Martchus commented 3 years ago

I guess I could already pin it down to the renderSvgImage() function which hasn't changed between the releases. If I'm running out of ideas I'll have to try bisecting. It would also be interesting to use GDB. Interestingly, I cannot reproduce the problem in WINE. (I haven't had much time to look into this so unfortunately I couldn't figure it out so far.)

Martchus commented 3 years ago

GDB actually shows a nice backtrace:

(gdb) bt
#0  0x00007ff829572346 in ntdll!RtlRaiseStatus ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#1  0x00007ff8294a0bb3 in ntdll!RtlUnwindEx ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#2  0x00007ff828322eed in msvcrt!_setjmpex ()
   from C:\WINDOWS\System32\msvcrt.dll
#3  0x000000000175ed50 in gray_record_cell ()
#4  0x000000000175ff74 in gray_render_scanline ()
#5  0x000000000175f5b0 in gray_render_line ()
#6  0x000000000175e58f in gray_convert_glyph_inner.constprop ()
#7  0x000000000175e147 in gray_convert_glyph ()
#8  0x0000000000fed812 in QRasterPaintEnginePrivate::rasterize(QT_FT_Outline_*, void (*)(int, QT_FT_Span_ const*, void*), void*, QRasterBuffer*) ()
#9  0x0000000000decf88 in QRasterPaintEngine::fill(QVectorPath const&, QBrush const&) ()
#10 0x0000000000bd3f2d in QPaintEngineEx::draw(QVectorPath const&) ()
#11 0x000000000120c2d7 in QSvgPath::draw(QPainter*, QSvgExtraStates&) ()
#12 0x0000000000cb7f3b in QSvgTinyDocument::draw(QPainter*, QRectF const&) ()
#13 0x00000000010cd56e in QPixmap Data::Detail::renderSvgImage<QByteArray>(QByteArray const&, QSize const&, int) ()
#14 0x000000000109f711 in Data::renderSvgImage(QByteArray const&, QSize const&, int) ()
#15 0x000000000083f7aa in runApplication(int, char const* const*) ()
#16 0x00000000007ffb92 in WinMain ()
#17 0x00000000004013c1 in __tmainCRTStartup ()
    at /build/mingw-w64-crt/src/mingw-w64-v9.0.0/mingw-w64-crt/crt/crtexe.c:321
#18 0x00000000004014d6 in WinMainCRTStartup ()
    at /build/mingw-w64-crt/src/mingw-w64-v9.0.0/mingw-w64-crt/crt/crtexe.c:176

(This one is with Qt 6 and Data::renderSvgImage is already been called early because I've modified the code to run the problematic function as soon as possible.) It is likely a miscompilation of Qt itself (likely Qt Gui, not the Svg module) which happens with GCC 11 and mingw-w64 9 or a regression withing Qt affecting versions 5 and 6.)

Martchus commented 3 years ago

It even crashes on a minimal example: https://gist.github.com/Martchus/e526dc09c81bc48770af9ebb8844460b

sledgehammer999 commented 3 years ago

I tested the minimal example on Windows 7 with msvc2017+qt5.15.2 (64bits). It doesn't crash. I launched it multiple times and it exited correctly.

I might be able to try with mingw 10.3 the next few days too.

sledgehammer999 commented 3 years ago

And these lines

#include <QtPlugin>
Q_IMPORT_PLUGIN(QWindowsIntegrationPlugin)

probably aren't needed.

Martchus commented 3 years ago

Ok. Btw, I'm not sure yet whether GCC 11 or mingw-w64 9.0.0 makes the difference but like to note that GCC and mingw-w64 have actually different versioning.

These lines are needed for using a static Qt build like the sample g++ command does. Otherwise it is not needed of course.

sledgehammer999 commented 3 years ago

Btw, I tested the minimal with gcc 10.3.0 (mingw-w64 from msys2) and it still works without crashing.

I started building the v1.1.7 tag but I am hitting a problem with the build process. Keep in mind I am basically a CMake noob. I've configured it and started compiling but at certain point it errors with this:

<path>/syncthingtray/model/syncthingrecentchangesmodel.h:6:10: fatal error: syncthingconnector/syncthingconnectionstatus.h: No such file or directory
    6 | #include <syncthingconnector/syncthingconnectionstatus.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated

What am I missing here? EDIT: The above is being tried inside an msys2 shell with cmake/ninja installed from msys2 inside the msys2 environment.

Martchus commented 3 years ago

Btw, I tested the minimal with gcc 10.3.0

Good to know. Maybe I'm building GCC 10.3.0 to compile Qt (at least base/svg) with it as a workaround.


Did you follow https://github.com/Martchus/syncthingtray#building-this-straight ? The includes rely on symlinks (see https://github.com/Martchus/syncthingtray/tree/master/include). Not sure whether Git handles symlinks correctly under Windows (NTFS generally supports them). And yes, this setup is a bit messy but I wanted to have these include paths resolvable in the same way during build of Syncthing Tray's libs and when one would consume the installed libs. (Maybe I should just rename the directories in Git to match the installed ones at some point.)

sledgehammer999 commented 3 years ago

The includes rely on symlinks

Yeah that's the problem. git creates these symlinks as files on the filesystem. I try to navigate through the git-bash but it doesn't work. Apparently I won't be able to pursue this further.

FIY, out of curiosity I tried with msvc2017. It craps out at commandlineutils.cpp: startConsole() and wherever _open_osfhandle is used. Apparently this is a function provided by the CRT. I assume that mingw exposes it directly but msvc needs the correct include. Just a FYI.

Also another advice: In c++utilities/cmake/modules/3rdParty.cmake try changing

set(CMAKE_REQUIRED_FLAGS -std=c++17)

to something more portable like:

set_property(TARGET my_target PROPERTY CXX_STANDARD 17)
set_property(TARGET my_target PROPERTY CXX_STANDARD_REQUIRED ON)

That's because in msvc the switch is /std:c++17

sledgehammer999 commented 3 years ago

I fixed the symlink problem:

  1. Run git bash as Administrator (needed for having symlink creation privileges)
  2. Clone with git clone -c core.symlinks=true https://github.com/Martchus/syncthingtray.git

Now, I am building it. I'll report back.

sledgehammer999 commented 3 years ago

It fails at linking with this:

C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/syncthingtray.dir/objects.a(main.cpp.obj):main.cpp:(.text.startup+0x1f): undefined reference to `qt_static_plugin_QSvgPlugin()'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/syncthingtray.dir/objects.a(main.cpp.obj):main.cpp:(.text.startup+0x2f): undefined reference to `qt_static_plugin_QSvgIconPlugin()'
collect2.exe: error: ld returned 1 exit status
make[2]: *** [syncthingtray/tray/CMakeFiles/syncthingtray.dir/build.make:706: syncthingtray/tray/syncthingtray.exe] Error 1
make[1]: *** [CMakeFiles/Makefile2:1215: syncthingtray/tray/CMakeFiles/syncthingtray.dir/all] Error 2
make: *** [Makefile:146: all] Error 2
Martchus commented 3 years ago

I've read your comments. I don't use MSVC myself so don't expect any improvements there. Feel free to create a PR, though.

Unless you're actually attempting a statically linked build it should not require these symbols and something must have been misconfigured. What were your CMake arguments? (And if you attempted a static build then it apparently failed to link against the static plugins for SVG support.)

Martchus commented 3 years ago

Also another advice: …

Not sure how that would work, though. The flag is supposed to be picked up by check_cxx_source_compiles. (For the actual targets I'm already using the CXX_STANDARD property.)

Martchus commented 3 years ago

Btw, I tested the minimal with gcc 10.3.0 (mingw-w64 from msys2) and it still works without crashing.

Was Qt itself built with that compiler version? That's what matters. (I'm asking because for other people 10.3.0 produces similar crashes: https://bugreports.qt.io/browse/QTBUG-93476)

Btw, I also created a Qt bug because I suspect some UB within Qt's code to be the culprit: https://bugreports.qt.io/browse/QTBUG-94692

sledgehammer999 commented 3 years ago

And if you attempted a static build then it apparently failed to link against the static plugins for SVG support.

Yes, it is a static build of qt that is used.

My CMake command is:

 cmake -DCMAKE_BUILD_TYPE=Release -DWEBVIEW_PROVIDER=none -DJS_PROVIDER=none -DNO_CLI=ON -G "MSYS Makefiles" -DCMAKE_PREFIX_PATH=<path-to-my-qt-build> -DCMAKE_INSTALL_PREFIX=../install ../subdirs/syncthingtray/


Was Qt itself built with that compiler version?

Yes, I freshly build it with that compiler from source.

Not sure how that would work, though. The flag is supposed to be picked up by check_cxx_source_compiles. (For the actual targets I'm already using the CXX_STANDARD property.)

I don't know much about CMake, so I won't comment further on this.

Martchus commented 3 years ago

Yes, it is a static build of qt that is used.

Are the plugins for svg icons present in your installation (and the CMake find modules correct)? Unfortunately using static plugins via CMake didn't work for quite a while because it wasn't really supported by upstream. Hence I added some patches in my own builds to fix it. Now Qt has improved its support upstream but it might be that there are still some bits missing. I'm still relying on some of my own patches when using Qt 5 and haven't tested upstream's way of handling things for Qt 5 (never touch a running system). You could also get rid of this by adding -DSVG_SUPPORT=OFF and -DSVG_ICON_SUPPORT=OFF to the CMake arguments which disables the plugins. The affected Svg code should still be present because it uses Qt Svg directly (and not the plugins).

I don't know much about CMake, so I won't comment further on this.

But you're right that the code is far from ideal. Maybe it helps to set CMake's global variables here. (Just for the record: https://github.com/Martchus/cpp-utilities/commit/2d50799cfffb57d394cf0b15cb7a985944572a08)

sledgehammer999 commented 3 years ago

Are the plugins for svg icons present in your installation (and the CMake find modules correct)?

The CMake configuration seems to finish ok. I don't see any output specific to the SVG plugin. Only about Qt::SVG. I can find plugins/imageformats/libqsvg.a in my qt installation path.

You could also get rid of this by adding -DSVG_SUPPORT=OFF and -DSVG_ICON_SUPPORT=OFF to the CMake arguments which disables the plugins.

I added it. The build finishes. The application starts and doesn't crash. But there are many missing icons. Apparently they are SVG and aren't loaded.

Martchus commented 3 years ago

See my comments on https://bugreports.qt.io/browse/QTBUG-94692 for my further investigation. I've been able to create patches for Qt 5 and Qt 6 to workaround the issue (links to the patches are referenced in the Qt bug).

Optimized builds of Syncthing Tray (latest Git master as of now) using GCC 11 and MinGW 9 with these patches applied can be found on my server:

You're welcome to test these builds. I've been testing them myself under the latest Windows 10 and could not reproduce crashes anymore. So I guess this is solved (although upstream Qt might not accept my patches).

Btw, I've just been removing the faulty Qt 5 build from the last release for now. Leaving only the Qt 6 build there should be good enough.

sledgehammer999 commented 3 years ago

Your test builds work on my machine now. Both qt5 and qt6. I tested the static 64bit version of syncthingtray only.

Btw, did you change something in the new release? Now a terminal window appears for a few seconds when syncthingtray is launched (double clicking on the exe).

Btw, I've just been removing the faulty Qt 5 build from the last release for now. Leaving only the Qt 6 build there should be good enough.

Will you now reinstated the qt5 build? Otherwise with qt6 you technically drop support for Windows below 10.

And I guess this issue can be closed, right?

Martchus commented 3 years ago

I've also tested the i686 build and they work as well. And yes, the issue can be closed.

Btw, did you change something in the new release? Now a terminal window appears for a few seconds when syncthingtray is launched (double clicking on the exe).

Yes, the internal launcher now uses Boost.Process (see https://github.com/Martchus/syncthingtray/issues/94) and that's apparently a side effect. Just start it with -no-console which has also been added to the suggested default parameters.

Will you now reinstated the qt5 build? Otherwise with qt6 you technically drop support for Windows below 10.

I'll continue building against Qt 5 so the next release will have the build available again. Note that I do not support any Windows version below 10 anyways in the sense that there's no testing done under older versions. However, I will not intentionally or needlessly break it. The same counts for Qt itself. That they don't support older Windows versions anymore doesn't mean it doesn't work (for now), they just don't invest any effort anymore into testing and preserve the possibility of actually breaking it in the future.

sledgehammer999 commented 3 years ago

Yes, the internal launcher now uses Boost.Process (see #94) and that's apparently a side effect. Just start it with -no-console which has also been added to the suggested default parameters.

I did but the console window still shows up. It might be coming from syncthingtray itself. Did you change anything else about console windows in tray?

sledgehammer999 commented 3 years ago

Btw, I think I figured why the official 1.1.7 qt6 didn't crash for me. Are you using a different icon set between qt5 and qt6 builds?

qt5: tray-qt5

qt6: tray-qt6

Martchus commented 3 years ago

I did but the console window still shows up. It might be coming from syncthingtray itself. Did you change anything else about console windows in tray?

I really don't see any consoles here with -no-console on Windows 10. And no, I didn't change much - except that you can now use ENABLE_CONSOLE=0 to disable the console Window of syncthingctl. It shouldn't have an impact on syncthingtray. Does Syncthing's -no-console switch work for you in general?

Btw, I think I figured why the official 1.1.7 qt6 didn't crash for me.

You mean the Qt 6 build from the GitHub release section for 1.1.7? I believe this one simply doesn't crash because it is still using Qt 6.1.0 which I have only built using GCC 10.2.0 and mingw-w64 8.0.0.

Are you using a different icon set between qt5 and qt6 builds?

For the devel builds I've mentioned I've been using the build scripts from https://github.com/Martchus/PKGBUILDs (it is the -DBUILTIN_ICON_THEMES:STRING=breeze;breeze-dark;Numix parameter). So no, it shouldn't use different icons. Which icon size is chosen might depend on Qt's way of scaling the icons and Qt 5 and Qt 6 have different default there. You can configure this via some env variables, though.