Bug 119856 - kde5: Java extension "WollMux" not working properly
Summary: kde5: Java extension "WollMux" not working properly
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
6.2.0.0.alpha0+
Hardware: All Linux (All)
: medium normal
Assignee: Katarina Behrens (Inactive)
URL:
Whiteboard: target:6.2.0 target:6.3.0 target:6.2.5
Keywords:
Depends on:
Blocks: KDE, KF5
  Show dependency treegraph
 
Reported: 2018-09-13 15:50 UTC by Michael Weghorn
Modified: 2019-05-22 11:46 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
backtrace for "Save as pdf" from WollMux form dialog (12.17 KB, text/plain)
2019-02-13 16:29 UTC, Michael Weghorn
Details
backtrace of 2 threads (28.58 KB, text/plain)
2019-02-21 17:17 UTC, Michael Stahl (allotropia)
Details
DEMO patch for approach in comment 20 (14.08 KB, patch)
2019-02-25 10:20 UTC, Michael Weghorn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Weghorn 2018-09-13 15:50:35 UTC
version: master build as of commit e95930d96459cc653342e78617db9498255569d0
VCL: kde5 on X11 (the gtk3 VCL plugin e.g. works fine)
platform: Debian testing, Plasma 5 desktop

(Note: It's not expected that any of the volunteers working on LibreOffice needs to deal more intensely with the details of the WollMux extension. This bug is mostly to keep track for those directly involved.)

Steps to reproduce:

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.

Expected result: Two dialogs/windows related to WollMux should come up ("Mail merge (WollMux)", "Select mail merge data") and the extension can be used to carry out mail merge.
Comment 1 Katarina Behrens (Inactive) 2018-09-14 15:26:45 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
Comment 2 Jan-Marek Glogowski 2018-09-21 06:33:16 UTC
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.
Comment 3 Commit Notification 2018-10-22 18:25:41 UTC
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.
Comment 4 Katarina Behrens (Inactive) 2018-10-22 18:29:48 UTC
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
Comment 5 Michael Weghorn 2018-10-22 21:11:07 UTC
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?
Comment 6 Katarina Behrens (Inactive) 2018-10-23 07:24:23 UTC
> 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
Comment 7 Katarina Behrens (Inactive) 2018-10-24 11:26:55 UTC
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
Comment 8 Michael Weghorn 2018-10-24 13:12:28 UTC
(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.
Comment 9 Commit Notification 2018-10-25 08:36:10 UTC
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.
Comment 10 Katarina Behrens (Inactive) 2018-10-25 08:36:40 UTC
This bug is like shaving yacs. I'll leave it open for a while
Comment 11 Commit Notification 2018-11-12 22:05:02 UTC
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.
Comment 12 Commit Notification 2018-11-12 22:06:24 UTC
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.
Comment 13 Xisco Faulí 2018-12-13 11:48:57 UTC
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
Comment 14 Xisco Faulí 2019-01-17 12:09:57 UTC
(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 ?
Comment 15 Michael Weghorn 2019-02-13 16:29:20 UTC
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)
Comment 16 Katarina Behrens (Inactive) 2019-02-20 20:35:49 UTC
 > 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
Comment 17 Michael Weghorn 2019-02-20 21:09:11 UTC
> 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.)
Comment 18 Katarina Behrens (Inactive) 2019-02-21 08:53:42 UTC
> 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
Comment 19 Michael Stahl (allotropia) 2019-02-21 17:17:47 UTC
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.
Comment 20 Katarina Behrens (Inactive) 2019-02-21 21:33:26 UTC
> 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
Comment 21 Michael Weghorn 2019-02-25 10:20:53 UTC
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.
Comment 22 Michael Weghorn 2019-02-25 10:35:12 UTC
For the record: mst_ was faster and created this WIP patch on gerrit: https://gerrit.libreoffice.org/#/c/68232/
Comment 23 Commit Notification 2019-03-05 11:49:21 UTC
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.
Comment 24 Commit Notification 2019-03-05 11:57:28 UTC
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.
Comment 25 Commit Notification 2019-03-05 12:11:11 UTC
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.
Comment 26 Commit Notification 2019-03-05 12:12:37 UTC
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.
Comment 27 Michael Weghorn 2019-03-06 11:48:29 UTC
The crash from comment 15 no longer happens with the above commits in place.
Comment 28 Katarina Behrens (Inactive) 2019-03-13 10:05:03 UTC
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
Comment 29 Michael Weghorn 2019-03-13 10:38:50 UTC
Verified as per comment 27.
Comment 30 Commit Notification 2019-05-22 11:45:52 UTC
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.
Comment 31 Commit Notification 2019-05-22 11:46:01 UTC
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.
Comment 32 Commit Notification 2019-05-22 11:46:09 UTC
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.
Comment 33 Commit Notification 2019-05-22 11:46:16 UTC
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.