| Summary: | kde5: Java extension "WollMux" not working properly | ||
|---|---|---|---|
| Product: | LibreOffice | Reporter: | Michael Weghorn <m.weghorn> |
| Component: | UI | Assignee: | Katarina Behrens (Inactive) <Katarina.Behrens> |
| Status: | VERIFIED FIXED | ||
| Severity: | normal | CC: | Katarina.Behrens, marsianer, michael.stahl, m.weghorn, thb, xiscofauli |
| Priority: | medium | ||
| Version: | 6.2.0.0.alpha0+ | ||
| Hardware: | All | ||
| OS: | Linux (All) | ||
| Whiteboard: | target:6.2.0 target:6.3.0 target:6.2.5 | ||
| Crash report or crash signature: | Regression By: | ||
| Bug Depends on: | |||
| Bug Blocks: | 102495 | ||
| Attachments: |
backtrace for "Save as pdf" from WollMux form dialog
backtrace of 2 threads DEMO patch for approach in comment 20 |
||
|
Description
Michael Weghorn
2018-09-13 15:50:35 UTC
> 1) set up the "WollMux" extension (including default configuration) as
> described at http://wollmux.org/ and restart LibreOffice Writer
> 2) select "Tools" -> "Mail merge (WollMux)"
>
> Result: Nothing seems to happen.
Well something *does* happen, I get this:
vcl/qt5/Qt5Menu.cxx:244: menu triggered 555
printed to stdout, but that's likely not what one would expect to happen
The strategy to fix this is to move all QWidget manipulation into the main thread. An example can be seen as the Qt5Frame part of commit b7d1371910da ("Qt5 minimal initial fix for Java UNO").
Now eventually all the GUI manipulation should go into the main thread, so we could get rid of the SolarMutex… probably not in my lifetime.
Katarina Behrens committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=a77163f0bd21d018da0e195974b42d6d71503592 tdf#119856: [Re-]activate menu before dispatching command It will be available in 6.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. With the above commit, I see 2 Java dialogs after clicking on Tools > MailMerge WollMux. Similarly, I can make About > About WollMux dialog show up. I tried clicking on some of the buttons in those dialogs, but certainly not all of them, so it is still possible that some of them ain't functional Cool, I get those dialogs as well now. One thing I quickly realized is that "Tools" -> "Mail merge (WollMux)" -> "File" (in the WollMux dialog) does not yet open a file picker. Does it make sense to handle this as part of this bug report or should I create a separate one? > One thing I quickly realized is that "Tools" -> "Mail merge (WollMux)" -> > "File" (in the WollMux dialog) does not yet open a file picker. This IS a different problem, in particular the one JMux mentions in comment 2 ... > Does it make sense to handle this as part of this bug report or should I > create a separate one? ... therefore let's keep thiz bug open I can open kde5 fpicker from WollMux now but I'm positive there's lot more threading landmines like this one hidden all over the place. I'd suggest to avoid scope-creep and open new tickets when (not 'if' :)) any further WollMux misbehaviour is found (In reply to Katarina Behrens (CIB) from comment #7) > I'd suggest to avoid scope-creep and open new tickets when (not 'if' :)) any > further WollMux misbehaviour is found Sound's reasonable. Feel free to mark this one as FIXED once the corresponding change has been merged to master. Katarina Behrens committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=8f71eb5182f7fe6d3e19705ed233f5f84a0a6208 tdf#119856 related: opening kde5 filepicker from extensions now possible It will be available in 6.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. This bug is like shaving yacs. I'll leave it open for a while Katarina Behrens committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/8fb0881a3e5b2c5120af18823f6f58a1bda7cadd%5E%21 tdf#119856: thread-proof kde5 fpicker execute() and getFiles() It will be available in 6.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Katarina Behrens committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/927f58bd997b6c1b60e354ee247a119d3f0af64b%5E%21 tdf#119856: thread-proof creating frames and setting menus It will be available in 6.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. A polite ping to Katarina Behrens: Is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Otherwise, Could you please explain what's missing? Thanks (In reply to Xisco Faulí from comment #13) > A polite ping to Katarina Behrens: > Is this bug fixed? if so, could you please close it as RESOLVED FIXED ? > Otherwise, Could you please explain what's missing? > Thanks Ping ? Created attachment 149276 [details] backtrace for "Save as pdf" from WollMux form dialog @bubli: As discussed on IRC, I did a quick retest, using master as of commit 49c21e31ce0501044a7d3602379f74c71dabb00b (+ 1 local commit that reverts a2b687a88feedfae0087bfc999b3cf49b9d3d24b to avoid bug 123439). Many things "just work" now. :) However, I get a crash for 1) start LibreOffice Writer 2) open WollMux sidebar -> "Formulare" -> "Externer Briefkopf als Formular" 3) wait until the document content has been initialized by WollMux and the form dialog is shown 4) click "Save as pdf" in WollMux form dialog s. attached backtrace (works fine with gtk3 VCL plugin) > However, I get a crash for
>
> 1) start LibreOffice Writer
> 2) open WollMux sidebar -> "Formulare" -> "Externer Briefkopf als Formular"
> 3) wait until the document content has been initialized by WollMux and the
> form dialog is shown
> 4) click "Save as pdf" in WollMux form dialog
>
> s. attached backtrace (works fine with gtk3 VCL plugin)
Sob, I can't repro ;-( Do I need a specific version of Wollmux? (mine is pretty old, 18.01)
The only thing that irritates the hell outta me in dual screen setup is that while Wollmux does its thing with this form document, the window keeps jumping from the right screen (where I usually work) to the left one. But I guess that's this other 'window won't remember its position and size' bug
> Sob, I can't repro ;-( Do I need a specific version of Wollmux? (mine is > pretty old, 18.01) I'm using WollMux 18.02 and "wollmux-config" of the same version, as provided on http://wollmux.org/ . Does the PDF export finish successfully for you? (I still do get some "PDF options" dialog and the WollMux form window remains there, but LibreOffice already crashed at that point and the recovery dialog is shown behind the "PDF options" dialog.) > I'm using WollMux 18.02 and "wollmux-config" of the same version, as
> provided on http://wollmux.org/ .
> Does the PDF export finish successfully for you?
Yep it does. I see messages like this on stderr:
Object::setParent: Cannot set parent, new parent is in a different thread
warn:vcl.gdi:12471:12471:vcl/source/graphic/Manager.cxx:135: Calculated size mismatch. Variable size is '272608' but calculated size is '136816'
warn:vcl.gdi:12471:12471:vcl/source/graphic/Manager.cxx:135: Calculated size mismatch. Variable size is '216616' but calculated size is '176716'
QObject::startTimer: Timers cannot be started from another thread
QObject::setParent: Cannot set parent, new parent is in a different thread
QObject::killTimer: Timers cannot be stopped from another thread
so some thread badness is certainly going on but whether it is the cause of the crash (and whether the crash goes away if it gets fixed), I can't tell
I guess I will read some Wollmux code to see what exactly is Wollmux doing there
Created attachment 149494 [details]
backtrace of 2 threads
thread 13 is loading UI file, one of the elements is GtkFrame "frame2" with 2 children "alignment2" and "label2".
while creating children of "alignment2", a "ComboBox" is created, which calls into "KDE5SalInstance::CreateFrame", which has:
if (!IsMainThread())
{
SolarMutexReleaser aReleaser;
return Q_EMIT createFrameSignal(pParent, nState);
}
this unblocks thread 1, and the first thing it does is run timers, which calls SystemWindow::ImplHandleLayoutTimerHdl, which does a layout of the dialog whose UI file hasn't finished loading in thread 13.
> thread 13 is loading UI file, one of the elements is GtkFrame "frame2" with
> 2 children "alignment2" and "label2".
Thread 13 will go to the naughty step because it's not supposed to be doing anything with UI (that includes loading UI files!), only main thread is allowed to do that.
So I wonder if reimplementing Qt5Instance::CreateBuilder with the familiar thread-proof construct:
if (!IsMainThread())
{
return Q_EMIT createBuilderSignal(some, arguments, here);
}
return SalInstance::CreateBuilder(some, arguments, here)
wouldn't help here. It would at least ensure that UI building is done in the main thread.
But I leave it up to mst_ or michaelweghorn bc I can't repro the bug myself sadly
Created attachment 149569 [details] DEMO patch for approach in comment 20 You mean a change like in the attached demo patch? With this one applied, another crash appears due to a failed assert related to the SolarMutex, s. bt in the commit message of the demo patch. (Line numbers might slightly differ, since the bt was from before making clang-format happy.) Same happens if I leave out the SolarMutexReleaser. For the record: mst_ was faster and created this WIP patch on gerrit: https://gerrit.libreoffice.org/#/c/68232/ Michael Stahl committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/265caa4381c048c346c907b017561ab0fe0367ff%5E%21 tdf#119856 vcl: Qt5/KDE5 RunInMainThread It will be available in 6.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Michael Stahl committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/621bebd8640feeb5f8e15fafac1425cb18bd28e4%5E%21 tdf#119856 vcl: fix Qt5FilePicker destructor to run in main thread It will be available in 6.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Michael Stahl committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/69c46bf53363127033a83c019375170550308ae2%5E%21 tdf#119856 vcl: fix Qt warning Qt5Frame::SetModal() It will be available in 6.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Michael Stahl committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/7e251025b97ed2a300270b1a565727441005c77e%5E%21 tdf#119856 vcl: convert Qt5FilePicker and Qt5Menu to RunInMainThread It will be available in 6.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. The crash from comment 15 no longer happens with the above commits in place. So let's close this as fixed, I wouldn't put last 4 patches into 6.2 though unless they are needed to resolve a critical bug Verified as per comment 27. Michael Stahl committed a patch related to this issue. It has been pushed to "libreoffice-6-2": https://git.libreoffice.org/core/+/5ad697fd5912cabf6fe645e15c85a86ceca3a158%5E%21 tdf#119856 vcl: Qt5/KDE5 RunInMainThread It will be available in 6.2.5. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Michael Stahl committed a patch related to this issue. It has been pushed to "libreoffice-6-2": https://git.libreoffice.org/core/+/c6f2f4e86c60fcd23a4ee513ce63404d65806981%5E%21 tdf#119856 vcl: fix Qt5FilePicker destructor to run in main thread It will be available in 6.2.5. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Michael Stahl committed a patch related to this issue. It has been pushed to "libreoffice-6-2": https://git.libreoffice.org/core/+/5be0bd7ca8d8ed890f3a95680a8c7710c473dcdf%5E%21 tdf#119856 vcl: fix Qt warning Qt5Frame::SetModal() It will be available in 6.2.5. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Michael Stahl committed a patch related to this issue. It has been pushed to "libreoffice-6-2": https://git.libreoffice.org/core/+/97ecb2af57c5aa469d9d5dff5a488d96220269dc%5E%21 tdf#119856 vcl: convert Qt5FilePicker and Qt5Menu to RunInMainThread It will be available in 6.2.5. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. |