Description: The migration routine declares table columns of type Binary[VARBINARY] from hsql into Text[VARCHAR] (according to Libo 6.0.3.2. If there is an image file stored in the first record this causes libo to become non-responsive and it must be forced to quit. stdout reads: (soffice:26591): GLib-GObject-WARNING **: 16:52:47.499: ../../../../gobject/gsignal.c:3492: signal name 'selection_changed' is invalid for instance '0x30b9390' of type 'OOoAtkObjCompTxt' double free or corruption (!prev) Steps to Reproduce: 1. download test file, open it 2. open form say later when it asks to migrate 3. form displayes graphic from table in image control 4. close the file, reopen it 5 open the form, answer yes to migration prompt Actual Results: system becomes non-responsive Expected Results: finish migration open form Reproducible: Always User Profile Reset: No Additional Info: tested w/build: Version: 6.1.0.0.alpha1+ Build ID: f1579d3d6c5f5f3a651825e035b93bee7a4f43c6 CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk2; TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-05-03_10:04:51 Locale: en-US (en_US.UTF-8); Calc: group User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/65.0.3325.181 Chrome/65.0.3325.181 Safari/537.36
Created attachment 141910 [details] test file with data single table, one record, single form
Could confirm the buggy behavior. LO crashes while migrating the database with Version: 6.1.0.0.alpha1+ Build-ID: 4ed3137022efa6128ad146e4b4dfae13548431dc CPU-Threads: 4; BS: Linux 4.4; UI-Render: Standard; VCL: kde4; TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-05-04_01:13:51 Gebietsschema: de-DE (de_DE.UTF-8); Calc: group Don't know why Binary isn't imported as BLOB - Binary. Nobody will use LONGVARCHAR in such a field for HSQLDB.
Created attachment 141938 [details] Test file using binary(fix)[BINARY] Data type of Binary(fix)[BINARY] will also cause Libo to become non responsive. This test file includes one record with data added via an image control. Output at stdout: (soffice:14582): GLib-CRITICAL **: 14:37:18.825: g_file_test: assertion 'filename != NULL' failed malloc(): smallbin double linked list corrupted If you open the file in hsql mode and delete the record then restart migration it runs to completion and the fb data type is set as: Text(fix)[CHAR] Test with build Version: 6.1.0.0.alpha1+ Build ID: f1579d3d6c5f5f3a651825e035b93bee7a4f43c6 CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk2; TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-05-03_10:04:51 Locale: en-US (en_US.UTF-8); Calc: group
Created attachment 142136 [details] valgrind trace On pc Debian x86-64 with master sources updated today, I could reproduce this. I also reproduced the pb when indicating Yes for the migration when opening form first time. I retrieved Valgrind trace.
Created attachment 142137 [details] bt with debug symbols
Created attachment 142167 [details] bt about length of varbinary on hsqldb Drew, what's the size of the image included in the table? In hsqldb, VARBINARY is 8000 bytes max (see https://www.tutorialspoint.com/hsqldb/hsqldb_data_types.htm) and https://opengrok.libreoffice.org/xref/core/dbaccess/source/filter/hsqldb/createparser.cxx#148 shows that indeed, we retain 8000 bytes. But it seems there are several varbinary types, images may be until 2,147,483,647 bytes! (see the same link) In gdb, breaking in /home/julien/lo/libreoffice/connectivity/source/drivers/firebird/PreparedStatement.cxx:844 gives: Thread 1 "soffice.bin" hit Breakpoint 2, connectivity::firebird::OPreparedStatement::setBytes (this=0x5555576bdcf0, nParameterIndex=2, xBytes=uno::Sequence of length 20897 = {...}) at /home/julien/lo/libreoffice/connectivity/source/drivers/firebird/PreparedStatement.cxx:839 844 Sequence<sal_Int8> xBytesCopy(xBytes); (gdb) p xBytes $1 = uno::Sequence of length 20897 = {-119 '\211', 80 'P', 78 'N', 71 'G', 13 '\r', 10 '\n', 26 '\032', 10 '\n', 0 '\000', 0 '\000', 0 '\000', 13 '\r', 73 'I', 72 'H', 68 'D', 82 'R', 0 '\000', 0 '\000', 3 '\003', 24 '\030', 0 '\000', 0 '\000', 0 '\000', -1 '\377', 8 '\b', 6 '\006', 0 '\000', 0 '\000', 0 '\000', 114 'r', -44 '\324', 112 'p', -61 '\303', 0 '\000', 0 '\000', 0 '\000', 4 '\004', 115 's', 66 'B', 73 'I', 84 'T', 8 '\b', 8 '\b', 8 '\b', 8 '\b', 124 '|', 8 '\b', 100 'd', -120 '\210', 0 '\000', 0 '\000', 0 '\000', 25 '\031', 116 't', 69 'E', 88 'X', 116 't', 83 'S', 111 'o', 102 'f', 116 't', 119 'w', 97 'a', 114 'r', 101 'e', 0 '\000', 103 'g', 110 'n', 111 'o', 109 'm', 101 'e', 45 '-', 115 's', 99 'c', 114 'r', 101 'e', 101 'e', 110 'n', 115 's', 104 'h', 111 'o', 116 't', -17 '\357', 3 '\003', -65 '\277', 62 '>', 0 '\000', 0 '\000', 32 ' ', 0 '\000', 73 'I', 68 'D', 65 'A', 84 'T', 120 'x', -100 '\234', -20 '\354', -35 '\335', 123 '{', 88 'X', 85 'U', -25 '\347', -99 '\235', -9 '\367', -1 '\377', -9 '\367', 62 '>', -80 '\260', 1 '\001', -35 '\335', 91 '[', 97 'a', -125 '\203', -70 '\272', 73 'I', 4 '\004', 52 '4', 5 '\005', 91 '[', -95 '\241', 26 '\032', 49 '1', -115 '\215', 76 'L', 19 '\023', 73 'I', -46 '\322', 104 'h', 77 'M', 49 '1', 83 'S', 113 'q', -110 '\222', 106 'j', 58 ':', -43 '\325', -103 '\231', -44 '\324', -76 '\264', 83 'S', 109 'm', -89 '\247', 58 ':', -45 '\323', -122 '\206', 103 'g', -6 '\372', -124 '\204', -10 '\366', 55 '7', 33 '!', -41 '\327', -92 '\244', -40 '\330', 105 'i', -75 '\265', -65 '\277', 62 '>', 58 ':', 79 'O', 11 '\v', 51 '3', -119 '\211', -74 '\266', -3 '\375', 73 'I', 103 'g', 18 '\022', 39 '\'', -87 '\251', -104 '\230', 70 'F', -90 '\246', 105 'i', 72 'H', -109 '\223', -112 '\220', -104 '\230', 66 'B', 90 'Z', -79 '\261', 90 'Z', 48 '0', 17 '\021', 26 '\032', 35 '#', -102 '\232', -128 '\200', 81 'Q', -74 '\266', 7 '\a', -74 '\266', 2 '\002', -101 '\233', 125 '}', -8 '\370', -3 '\375', -63 '\301', 65 'A', 78 'N'...} (gdb) so we got here a sequence of 20897 values and since they're bytes, 20897 bytes ; to compare with the 8000 bytes allocated...
Here's a bandaid patch: iff --git a/connectivity/source/drivers/firebird/PreparedStatement.cxx b/connectivity/source/drivers/firebird/PreparedStatement.cxx index 0a43d5c93851..61adf79b5637 100644 --- a/connectivity/source/drivers/firebird/PreparedStatement.cxx +++ b/connectivity/source/drivers/firebird/PreparedStatement.cxx @@ -843,6 +843,11 @@ void SAL_CALL OPreparedStatement::setBytes(sal_Int32 nParameterIndex, xBytesCopy.realloc( nMaxSize ); } const short nSize = xBytesCopy.getLength(); + if (nSize > 8000) + { + free(pVar->sqldata); + pVar->sqldata = static_cast<char *>(calloc(nSize + 2, sizeof(char))); + } memcpy(pVar->sqldata, &nSize, 2); // Actual data memcpy(pVar->sqldata + 2, xBytesCopy.getConstArray(), nSize); I use calloc to be sure not letting garbage in allocated memory. With this, it doesn't crash but I don't see the image in the form. BTW, I unzipped the odb file and don't see where the image is. Lionel/Tamas: any thoughts about this patch?
I forgot to tell, I was investigating about where the value 8000 came from because it's used here: 285 case SQL_VARYING: 286 pVar->sqldata = static_cast<char *>(malloc(sizeof(char)*pVar->sqllen + 2)); (see https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/firebird/Util.cxx#286) gdb indicated that pVar->sqllen = 8000 here.
I created the test file with huge values for those columns 2 Million bytes. HSQL embedded in Base works just fine with that. The images were 20 K and 32 K in size, added by linking an image control in a form to them and using that to insert the image. The forms work as expected with the data and reports work with it also. The reason for a 20k and a 32K file was that I thought Firebird was going to impose a 32K limit for the fixed field. My plan was to first test the migration function with empty tables (seemed to pass) then with this data that would fit into FB and when this passes a test migration , check for error handling when the data exceeds the FB imposed limits. As for the 8000 byte limit, IIRC, that is for if you want to index that column or if you are using a character set that supports collating.
some more information. In the current code the migration assistant ALWAYS, when it finds a varbinary, column in the import file, creates the firebird equivalent field with a size of 8000 bytes (even though firebird supports up to 32k in size for this type of field and I'll open a separate issue for that). // see earlier comments The test file attached to this issue includes a column with 20k of data. Running tests with 2K of data in the column did not trigger the hang, running a test with 9k data in the column did. The Base HSQLdb UI implementation doesn't allow the user to explicitly set the size of a varbinary field. The field size grows to accommodate the largest data value put into it, up to 2Meg (or is it 2Gig?) in size. Changing the summary on this issue to just deal with what happens when the migration assistant tries to put 20k into a 8k sack ;)
For information, I submitted for review on gerrit the patch quoted in comment 7, see https://gerrit.libreoffice.org/#/c/54863/
Excellent So there is two new problems here: First in firbird that column could be up to 32K in size ( and max data would be 32k - 2 bytes IIRC) Second - with the current migration assistant it creates a column type that is unrecognized and unusable by any current Base data control. The user can connect to a text control or an image control, either way the data is either lost when you try to insert it, mangled or irretrievable. However, if you do connect an image control to one of these migration assistant created column and open an image less then 8000 bytes in size it appears to write something to the table. If you try to export that data, to Writer for example, what you get is that value x0 and nothing else.
Julien Nabet committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=54ddc4ff4c2ff7e8b2c502d6b475cfdc9b8e3cec tdf#117446: FB mig, (VAR)BINARY, fix memory management 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.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-6-1": http://cgit.freedesktop.org/libreoffice/core/commit/?id=570279baa7df076413cc6b4f9220a07248db0c1b&h=libreoffice-6-1 tdf#117446: FB mig, (VAR)BINARY, fix memory management It will be available in 6.1.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.
LibreOffice doesn't hang in Version: 6.1.0.0.beta1+ Build ID: 8b96445766efe237eb47608ade6c147673466e2e CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); Calc: group threaded @Julien, @Drew Jensen, Since this issue is about Libreoffice hanging, should it be closed as RESOLVED FIXED ?
yes, the attached file and two others which did previously no longer do. test with Version: 6.1.0.0.beta1+ Build ID: b57819201ff89bbde14b4df8c02a0eb4bea2c2bb Branch:libreoffice-6-1, Time: 2018-06-04_04:41:46
With Version: 6.2.0.0.alpha0+ Build ID: c4c56de1b0e62ec866b519b2b24c5e805f0a86d3 CPU threads: 2; OS: Linux 4.4; UI render: default; VCL: gtk2; TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-06-09_00:23:35 Locale: en-US (en_US.UTF-8); Calc: threaded it doesn't crash anymore, but the patch has some side effects. The picture from attachment 141910 [details] is gone now if you migrate. And if you add a new image (any), it will also disappear when closing / reopening form 'tbl_varbinary'.
(In reply to MM from comment #17) > ... > it doesn't crash anymore, but the patch has some side effects. > The picture from attachment 141910 [details] is gone now if you migrate. And > if you add a new image (any), it will also disappear when closing / > reopening form 'tbl_varbinary'. Difficult to tell if it's a side effect since before the patch, there was a hang (or I missed something). I had noticed the disappearing of image too but I don't know why. At least the patch fixed the hang.
(In reply to MM from comment #17) > With Version: 6.2.0.0.alpha0+ > Build ID: c4c56de1b0e62ec866b519b2b24c5e805f0a86d3 > CPU threads: 2; OS: Linux 4.4; UI render: default; VCL: gtk2; > TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: > 2018-06-09_00:23:35 > Locale: en-US (en_US.UTF-8); Calc: threaded > > it doesn't crash anymore, but the patch has some side effects. > The picture from attachment 141910 [details] is gone now if you migrate. And > if you add a new image (any), it will also disappear when closing / > reopening form 'tbl_varbinary'. Right, but this issue was just about the application becoming non-responsive, that is fixed. The image disappearing is a separate issue, but even that is ancillary. There is a question as to how valid the test file is. Not that you can't store an image that way in the hsqldb version of Base, the file itself shows you can, but whether it is how anyone is using it. (it would be less than optimal performance wise). These are two datatypes not directly found in the Firebird engine, and the native firedbird datatype used in the new Firebird schema doesn't currently support moving data in and out via an image control. But again that is a separate issue, and someone should open it. ;-)