Bug 121838 - Firebird: Mirgration: Certain ODB crashes application during Migration Assistant execution
Summary: Firebird: Mirgration: Certain ODB crashes application during Migration Assist...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.2.0.0.beta1+
Hardware: All All
: high major
Assignee: Caolán McNamara
URL:
Whiteboard: target:6.2.0.2 target:6.3.0 target:6.1.5
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Database-Firebird-Migration
  Show dependency treegraph
 
Reported: 2018-12-01 10:50 UTC by Drew Jensen
Modified: 2019-01-10 08:55 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
File which crashes application during MA (6.39 MB, application/vnd.oasis.opendocument.database)
2018-12-01 10:50 UTC, Drew Jensen
Details
Second ODB which causes crash (21.51 KB, application/vnd.oasis.opendocument.database)
2018-12-01 16:31 UTC, Drew Jensen
Details
Test-database from Base-Handbook (937.74 KB, application/vnd.oasis.opendocument.database)
2018-12-19 19:17 UTC, Robert Großkopf
Details
gdb bt (13.79 KB, text/plain)
2018-12-20 18:05 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Drew Jensen 2018-12-01 10:50:09 UTC
Created attachment 147189 [details]
File which crashes application during MA

Checked with Ubuntu 18.04 (64 bit) and

LO 6.1.4 (GTK2)
The application seems to go into a deadlock, will prompt whether to 'Force quit' the application or 'wait' for it to return. Eventually you will select end. Upon restart it starts a send crash report dialog but without any details on the crash report.

LO 6.2Beta1 and 6.3Alpha0 (GEN, GTK2, GTK3)
The application when VCL = GEN or GTK3 displays the following error, and send it to stdout, during the migration assistant run: LibreOfficeDev 6.3 - Fatal Error: map::at. When you close the error dialog the application ends, upon restart it starts the crash reporter but again with no crash report details in the dialog.


Do the following to trigger the bug.

1 - download the attached file (TTNorthWind.odb)
2 - Open the file in Base and go to the Tables section
3 - Start the Migration Assistant when prompted.
4 - If you are using 6.2/6.3 close the error dialog which will appear 
crash

Expected result
Normal cleanup after failed migration.
Comment 1 Drew Jensen 2018-12-01 10:56:52 UTC
Using Ubuntu 18.04 and LO 6.1.3

The migration assistant generates the following error code:
firebird_sdbc error:
*unsuccessful metadata update
*ALTER TABLE Suppliers failed
*SQL error code = -607
*Invalid command
*Table Suppliers does not exist
caused by
'ALTER TABLE "Suppliers" ALTER COLUMN "SupplierID" RESTART WITH 29'

as the first of seven errors.

Closing the dialog box does not end in a crash.

The migration assistant cleans up after the error and displays the ODB with the tables it could migrate now in the firebird database engine.
Comment 2 MM 2018-12-01 13:55:53 UTC
Confirmed on mint 18.3 with Version: 6.3.0.0.alpha0+
Build ID: d8d231f97d829350d965105e3a5be119d1a6494c
CPU threads: 2; OS: Linux 4.15; UI render: default; VCL: gtk3; 
TinderBox: Linux-rpm_deb-x86_64@86-TDF, Branch:master, Time: 2018-11-30_00:14:18
Locale: en-US (en_US.UTF-8); UI-Language: en-US
Calc: threaded

It also crashes when trying to migrate Queries / Forms.
Comment 3 Drew Jensen 2018-12-01 16:31:24 UTC
Created attachment 147204 [details]
Second ODB which causes crash

Adding a second example file.
Much smaller and simpler.
All reports, forms and queries have been removed, leaving only the data schema.
Triggers the same crash as the other file.
Comment 4 Xisco Faulí 2018-12-04 08:28:17 UTC
Regression introduced by:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=76279399f747548494a259173ef5669553c3f06f

author	Tamas Bunth <tamas.bunth@collabora.co.uk>	2018-10-26 21:04:09 +0200
committer	Tamás Bunth <btomi96@gmail.com>	2018-10-29 10:02:56 +0100
commit 76279399f747548494a259173ef5669553c3f06f (patch)
tree 9efa72d1fb79472e4c868c7efeabc85c1206dbe8
parent a1dd2f55f24da4fa875a92eebf05c154528e1d74 (diff)
tdf#120691 Migrate tables which contain spaces

Bisected with: bibisect-linux64-6.2

Adding Cc: to Tamas Bunth
Comment 5 Robert Großkopf 2018-12-19 19:17:26 UTC
Created attachment 147678 [details]
Test-database from Base-Handbook

Have changed the views of the database of Base-Handbook to code which works in HSQLDB and Firebird. Deleted views, which won't work (moved them to queries). But no chance: If I start the database and switch to the tables the migration dialog appears and after some seconds LO crashes completely.
Comment 6 Julien Nabet 2018-12-20 18:05:03 UTC
Created attachment 147721 [details]
gdb bt

On pc Debian x86-64 with master sources updated today, I could reproduce the crash with file proposed by Robert (since he had cleaned the initial file).
Comment 7 Julien Nabet 2018-12-20 18:59:25 UTC
Trying to debug, I get in connectivity/source/drivers/firebird/PreparedStatement.cxx:841
during test, nSize = -1 because short type seems to small.

diff --git a/connectivity/source/drivers/firebird/PreparedStatement.cxx b/connectivity/source/drivers/firebird/PreparedStatement.cxx
index e0d120d0cfec..57cb70b7f178 100644
--- a/connectivity/source/drivers/firebird/PreparedStatement.cxx
+++ b/connectivity/source/drivers/firebird/PreparedStatement.cxx
@@ -847,7 +847,7 @@ void SAL_CALL OPreparedStatement::setBytes(sal_Int32 nParameterIndex,
             {
                 xBytesCopy.realloc( nMaxSize );
             }
-            const short nSize = xBytesCopy.getLength();
+            const sal_Int32 nSize = xBytesCopy.getLength();
             // 8000 corresponds to value from lcl_addDefaultParameters
             // in dbaccess/source/filter/hsqldb/createparser.cxx
             if (nSize > 8000)

prevents from having the crash.

Of course, it doesn't prevent from having the error message:
warn:connectivity.firebird:1966:1966:connectivity/source/drivers/firebird/Util.cxx:55: firebird_sdbc error:
*Dynamic SQL Error
*SQL error code = -104
*Token unknown - line 1, column 84
*\
caused by
'isc_dsql_prepare'

warn:connectivity.firebird:1966:1966:connectivity/source/drivers/firebird/Util.cxx:55: firebird_sdbc error:
*Dynamic SQL Error
*SQL error code = -104
*Name longer than database column size
caused by
'isc_dsql_prepare'

Lionel/Tamas: are you ok with the proposed patch above?
Comment 8 Caolán McNamara 2018-12-21 13:25:01 UTC
bug attached in comment #5, that comments #6 and #7 are relevant to, is a duplicate of bug #120576 and I came up with basically the same patch Julien suggests for that. The other crasher std::vector::at in the earlier .odbs remains
Comment 9 Caolán McNamara 2018-12-21 13:44:18 UTC
https://gerrit.libreoffice.org/65545 to solve the initial problem of a std::out_of_range on looking up column name which seems to happen because of a space in the column name
Comment 10 Commit Notification 2018-12-21 16:06:56 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-2":

https://git.libreoffice.org/core/+/2c0c0794a8e287497e460f3f1e6bcba585d675d4%5E%21

Resolves: tdf#121838 catch exception for missing column

It will be available in 6.2.0.2.

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 11 Commit Notification 2018-12-21 17:15:24 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/a63bdb1fb45b5a73fee6c826d302e0ce92876ece%5E%21

Resolves: tdf#121838 catch exception for missing column

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 12 Caolán McNamara 2018-12-21 17:17:22 UTC
backports to 6-2 and 6-1 in gerrit
Comment 13 Julien Nabet 2018-12-21 17:19:00 UTC
(In reply to Caolán McNamara from comment #8)
> bug attached in comment #5, that comments #6 and #7 are relevant to, is a
> duplicate of bug #120576 and I came up with basically the same patch Julien
> suggests for that. The other crasher std::vector::at in the earlier .odbs
> remains

Just to be sure, should I submit the patch concerning nSize on gerrit or do you think it'd be useless or wrong?
Comment 14 Caolán McNamara 2018-12-23 15:08:16 UTC
Your patch was good, but (not knowing about it here) I duplicated that work and backported it for 6-2 and 6-1, with the 6-1 one (https://gerrit.libreoffice.org/#/c/65540/1) still awaiting a review at the time of writing.
Comment 15 Xisco Faulí 2018-12-24 14:28:23 UTC
No longer crashes in

Version: 6.3.0.0.alpha0+
Build ID: 9520378e37b97b0a44130c86be482060465b479e
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Caolán, thanks for fixing this!!

@Drew, do you know the reason why it gives an error now ? Should it be reported in a different ticket ?
Comment 16 Drew Jensen 2018-12-24 17:26:26 UTC
(In reply to Xisco Faulí from comment #15)
> No longer crashes in
> 
> Version: 6.3.0.0.alpha0+
> Build ID: 9520378e37b97b0a44130c86be482060465b479e
> CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
> Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
> Calc: threaded
> 
> @Caolán, thanks for fixing this!!
> 
> @Drew, do you know the reason why it gives an error now ? Should it be
> reported in a different ticket ?

@Xisco Well, not really - but I just installed a daily build of 6.3 with both of the patches applied and have a half dozen odb files here which caused the crash. Will give them a go and see about doing as Robert did by decomposing them as I go looking for what might be triggering the error(s) - will get back on that, but not till Wednesday ;-) Merry Christmas
Comment 17 Commit Notification 2019-01-10 08:55:06 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

https://git.libreoffice.org/core/+/80dd8d8c2f7f6a14f20795757a5b3556caae1608%5E%21

Resolves: tdf#121838 catch exception for missing column

It will be available in 6.1.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.