Bug 91715 - Database wizard crashes when invoked through remote UNO
Summary: Database wizard crashes when invoked through remote UNO
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
5.0.0.0.alpha0+ Master
Hardware: Other All
: medium major
Assignee: Michael Meeks
URL:
Whiteboard: target:5.1.0 target:5.0.1
Keywords: bibisected, bisected, haveBacktrace, regression
Depends on:
Blocks: Database-Wizard
  Show dependency treegraph
 
Reported: 2015-05-29 06:58 UTC by Matthew Francis
Modified: 2017-10-23 20:42 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Reproduction script (720 bytes, text/x-python-script)
2015-05-29 07:05 UTC, Matthew Francis
Details
Linux 5.1 master backtrace (9.53 KB, text/plain)
2015-05-29 07:09 UTC, Matthew Francis
Details
console+bt with debug symbols (11.01 KB, text/plain)
2015-05-31 08:45 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Francis 2015-05-29 06:58:03 UTC
Invoking the database wizard through a remote UNO connection ends in a crash when the wizard ends (whether through completion or cancellation).

See attached script for reproducer


(even previously, the UNO call to invoke the wizard never returns until the wizard ends, which is unusual - probably nobody ever noticed this because the occasions when you'd want to invoke the wizard from code are limited)
Comment 1 Matthew Francis 2015-05-29 07:05:38 UTC
Created attachment 116121 [details]
Reproduction script

Reproduction steps:

With a master build,

- In one terminal:
instdir/program/soffice --accept="socket,host=localhost,port=2002;urp;" --invisible

- In another terminal:
UNO_PATH=$PWD/instdir ./tdf91715.py

- Then, when the database wizard appears, either cancel it or finish it
Comment 2 Matthew Francis 2015-05-29 07:08:25 UTC
Setting to NEW as a commit (range) has been identified:

This began somewhere in the range 9e678c14e4fc8e58b1e0530744f648fa3958d338..d05a64df34fd143670cb939b72abfb32d6b714c7
which is the Timer/Idle work

Adding Cc: to tobias.madl.dev@gmail.com; Could you possibly take a look at this one? Thanks
Comment 3 Matthew Francis 2015-05-29 07:09:00 UTC
Created attachment 116122 [details]
Linux 5.1 master backtrace
Comment 4 Matthew Francis 2015-05-29 07:43:59 UTC
Small error in the instructions in comment 1 - to get the correct pyuno installation with system python, the environment variables to set (from within your build directory) to run ./tdf91715.py are:

PYTHONPATH=$PWD/instdir/program
URE_BOOTSTRAP="vnd.sun.star.pathname:$PWD/instdir/program/fundamentalrc"
Comment 5 Julien Nabet 2015-05-31 08:45:01 UTC
Created attachment 116190 [details]
console+bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this.

I attached console logs+bt
Indeed, I noticed this:
warn:vcl:15664:10:vcl/generic/app/geninst.cxx:119: CheckYieldMutex: 1!=10
Comment 6 Julien Nabet 2015-05-31 08:45:44 UTC
Noticing VclPtr part, I put this one as blocker for tdf#91310
Comment 7 Michael Meeks 2015-06-01 08:39:15 UTC
Hmm; the error here is:

#2  0x00007efe51a4cb86 in __assert_fail_base (fmt=0x7efe51b9d830 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    assertion=assertion@entry=0x7efe4b9f1990 "ImplGetSVData()->mpDefInst->CheckYieldMutex() && \"SolarMutex not locked\"", 
    file=file@entry=0x7efe4b9f1948 

That looks like a pretty normal UNO / threading issue that is unrelated to VclPtr; and appears to be related to the new timer/idle work.

I guess we are simply not holding the SolarMutex while we're cleaning up all this VCL stuff. Then again - UNO & threading is a horrendous tangled mess from beginning to end.

To fix the assert; it would be worth trying the patch below - through 'make check' and also your use-case.

diff --git a/svtools/source/uno/genericunodialog.cxx b/svtools/source/uno/genericunodialog.cxx
index 4b7f82f..6cabb2b1 100644
--- a/svtools/source/uno/genericunodialog.cxx
+++ b/svtools/source/uno/genericunodialog.cxx
@@ -313,6 +313,7 @@ void SAL_CALL OGenericUnoDialog::initialize( const Sequence< Any >& aArguments )
 
 void OGenericUnoDialog::destroyDialog()
 {
+    SolarMutexGuard aSolarGuard;
     m_pDialog.disposeAndClear();
 }
 

Can you confirm that works ? if so, lets get it in.
Comment 8 Julien Nabet 2015-06-01 18:07:07 UTC
Michael: I gave a try to the patch you proposed, it seems it doesn't crash anymore with this! :)
However, here's what I get on console after having clicked on Cancel button:
warn:legacy.osl:3186:15:svtools/source/uno/genericunodialog.cxx:323: OGenericUnoDialog::OnDialogDying: where does this come from?
warn:sal.osl:3186:13:sal/osl/unx/socket.cxx:1800: receive socket [0] failed: EOL
warn:binaryurp:3186:13:binaryurp/source/reader.cxx:123: caught UNO exception 'acc_socket.cxx:SocketConnection::read: error - Success'
warn:binaryurp:3186:13:binaryurp/source/bridge.cxx:846: undisposed bridge, potential deadlock ahead

But perhaps it's normal (after all, they're just warnings).
Comment 9 Michael Meeks 2015-07-14 11:57:59 UTC
Michael - any thoughts of my banal but potentially correct patch to GenericUnoDialog ? =)
Comment 10 Commit Notification 2015-07-14 17:55:38 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0a7375e372ee9583d31d44a7cc7b6a21e6197bf1

tdf#91715: lock SolarMutex from dbaui::~ODatabaseAdministrationDialog()

It will be available in 5.1.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 11 Commit Notification 2015-07-14 17:59:27 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0fb49c071f8f6b1569a8cf20f7fc598b098b1eb5&h=libreoffice-5-0

tdf#91715: lock SolarMutex from dbaui::~ODatabaseAdministrationDialog()

It will be available in 5.0.1.

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 12 Julien Nabet 2015-07-14 19:04:23 UTC
Considering Michael's patches, let's consider this one as FIXED.
Comment 13 Robinson Tryon (qubit) 2015-12-17 09:13:10 UTC Comment hidden (obsolete)