Bug 119615 - CRASH - closing table causes crash using mysql ODB with native connector connected to a mysql instance
Summary: CRASH - closing table causes crash using mysql ODB with native connector conn...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.2.0.0.alpha0+
Hardware: All macOS (All)
: high major
Assignee: Not Assigned
URL:
Whiteboard: target:6.2.0
Keywords: haveBacktrace, regression
Depends on:
Blocks:
 
Reported: 2018-08-31 06:56 UTC by Alex Thurgood
Modified: 2018-08-31 13:47 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Backtrace from lldb session (9.78 KB, text/plain)
2018-08-31 07:21 UTC, Alex Thurgood
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Thurgood 2018-08-31 06:56:57 UTC
Description:
1) Load an ODB file configured to connect to a mysql server with the native connector

2) Open a table (double-click on any table from list)

3) Close the table window by mouse button clicking on the close button

4) Crash - recovery dialog started


Steps to Reproduce:
See above

Actual Results:
LibreOffice crashes and starts recovery dialog.

Expected Results:
LibreOffice should not crash. The table window should close and bring user back to main ODB application window.


Reproducible: Always


User Profile Reset: No



Additional Info:
Comment 1 Alex Thurgood 2018-08-31 06:58:21 UTC
I don't seem to be able to get a backtrace on this. Using lldb, when LO crashes, an attempt to get a backtrace reports that no thread is available, yet LibreOffice is still active in the OSX Dock (hence the recovery dialog).
Comment 2 Julien Nabet 2018-08-31 07:04:18 UTC
Tamas: thought it may interest you since it concerns Mysql.

Alex: I'll try to test this but must reinstall everything about Mysql (MariaDB?)/Apache.
Just for curiosity, do you reproduce this also with MariaDB or is Mysql only?
Comment 3 Alex Thurgood 2018-08-31 07:10:16 UTC
Hi Julien,

I don't have an accessible instance of mariadb with which to try this.

This is what I see in the lldb session up to the point where LO crashes:

Process 94260 launched: '/Users/Shared/LO/core/instdir/LibreOfficeDev.app/Contents/MacOS/soffice' (x86_64)
objc[94260]: Class FIFinderSyncExtensionHost is implemented in both /System/Library/PrivateFrameworks/FinderKit.framework/Versions/A/FinderKit (0x7fffb1864c90) and /System/Library/PrivateFrameworks/FileProvider.framework/OverrideBundles/FinderSyncCollaborationFileProviderOverride.bundle/Contents/MacOS/FinderSyncCollaborationFileProviderOverride (0x181ae7cd8). One of the two will be used. Which one is undefined.
2018-08-31 09:05:42.477886+0200 soffice[94260:12577831] MessageTracer: load_domain_whitelist_search_tree:73: Search tree file's format version number (0) is not supported
2018-08-31 09:05:42.477926+0200 soffice[94260:12577831] MessageTracer: Falling back to default whitelist
2018-08-31 09:05:42.999018+0200 soffice[94260:12577883] errors encountered while discovering extensions: Error Domain=PlugInKit Code=13 "query cancelled" UserInfo={NSLocalizedDescription=query cancelled}
soffice(94260,0x7fffb4cd6380) malloc: *** error for object 0x10950ec18: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Process 94260 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00007fff7c312b66 libsystem_kernel.dylib`__pthread_kill + 10
libsystem_kernel.dylib`__pthread_kill:
->  0x7fff7c312b66 <+10>: jae    0x7fff7c312b70            ; <+20>
    0x7fff7c312b68 <+12>: movq   %rax, %rdi
    0x7fff7c312b6b <+15>: jmp    0x7fff7c309ae9            ; cerror_nocancel
    0x7fff7c312b70 <+20>: retq   
Target 0: (soffice) stopped.
Comment 4 Alex Thurgood 2018-08-31 07:11:07 UTC
Note that for comparison this doesn't happen with hsqldb ODB file.
Comment 5 Alex Thurgood 2018-08-31 07:13:52 UTC
Ahaa ! Turns out that I did manage to get a bt, but I had to force break the lldb session to get it to display on screen. Will attach it here.
Comment 6 Alex Thurgood 2018-08-31 07:21:53 UTC
Created attachment 144567 [details]
Backtrace from lldb session
Comment 7 Alex Thurgood 2018-08-31 07:51:56 UTC
I can reproduce the same behaviour connecting to a mariadb instance with

Version: 6.2.0.0.alpha0+
Build ID: 0b06762ff19a804d3b86167ae3012811662412f1
CPU threads: 4; OS: Mac OS X 10.13.6; UI render: default; 
Locale: fr-FR (fr_FR.UTF-8); Calc: threaded
Comment 8 Julien Nabet 2018-08-31 08:40:29 UTC
Thank you Alex for the bt. Let's put this one to NEW then.

Noel: considering Alex's bt, I wonder if it could be due to https://cgit.freedesktop.org/libreoffice/core/commit/?id=48baa68e3ac12bbaf0d5b7912a0197accf341b25
It seems memory management may be tricky if we must synchronize LO memory and Mysql/MariaDb memory.
Comment 9 Noel Grandin 2018-08-31 09:11:36 UTC
@julien I agree, that commit is faulty, m_aFields should not have been converted to std::unique_ptr. Want to fix?
Comment 10 Julien Nabet 2018-08-31 09:30:36 UTC
(In reply to Noel Grandin from comment #9)
> @julien I agree, that commit is faulty, m_aFields should not have been
> converted to std::unique_ptr. Want to fix?

If it's just reverting m_aFields part of the patch, I'll be able to do this when I come back to home after my day time job.

However, it seems we may have a memory leak.
According to https://mariadb.com/kb/en/library/mysql_free_result/, a call to mysql_fetch_row implies a call to mysql_free_result and I'm not sure "OPreparedResultSet::close()" method was called in the bt.
Should a call to this method in destructor could make it? (and we could keep unique_ptr in the same time)?
Comment 11 Noel Grandin 2018-08-31 09:34:18 UTC
(In reply to Julien Nabet from comment #10)
> If it's just reverting m_aFields part of the patch, I'll be able to do this
> when I come back to home after my day time job.

Yup, that was what I meant.

> 
> However, it seems we may have a memory leak.
> According to https://mariadb.com/kb/en/library/mysql_free_result/, a call to
> mysql_fetch_row implies a call to mysql_free_result and I'm not sure
> "OPreparedResultSet::close()" method was called in the bt.
> Should a call to this method in destructor could make it? (and we could keep
> unique_ptr in the same time)?

Probably close() should call mysql_free_result and then set the pointer to null.
The destructor should check the pointer and only call mysql_free_result if the pointer is non-null.
Comment 12 Julien Nabet 2018-08-31 09:49:20 UTC
Thank you Noel for your new feedback.

Here what I'll do:
- reproduce this
- give a try with adding close method unconditionally in destructor but letting unique_ptr
=> Either I still reproduce the crash and I'll additionnaly revert unique_ptr part for m_aFields or it means calling close method is sufficient.
Obviously the goal is to fix the crash of course but also to avoid memory leaks, that's why I'd like to add this close method in any cases.
Comment 13 Commit Notification 2018-08-31 13:33:58 UTC
Tamas Bunth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9b75c54aa5b619c704536df2912cdab7e2241443

tdf#119615 Ownership of m_aFields is..

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 14 Julien Nabet 2018-08-31 13:47:55 UTC
Since Tamas fixed this one and removed unique_ptr, I suppose it was needed and everything should be ok by now.