Bug 140061 - Crash swlo!sw::WriterMultiListener::StartListening
Summary: Crash swlo!sw::WriterMultiListener::StartListening
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.3.0.4 release
Hardware: All All
: medium normal
Assignee: Patrick (volunteer)
URL:
Whiteboard: target:25.2.0 target:24.2.6 target:24...
Keywords: bibisectRequest, haveBacktrace, regression
Depends on:
Blocks: Crash
  Show dependency treegraph
 
Reported: 2021-02-01 13:57 UTC by Telesto
Modified: 2024-10-17 13:22 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Example file (208.87 KB, application/vnd.oasis.opendocument.text)
2021-02-01 13:57 UTC, Telesto
Details
BT without symbols (13.11 KB, text/plain)
2021-02-01 14:01 UTC, Telesto
Details
bt with debug symbols (12.56 KB, text/plain)
2021-02-01 20:10 UTC, Julien Nabet
Details
macOS crash log (148.67 KB, text/plain)
2024-08-04 22:32 UTC, Patrick (volunteer)
Details
Test patch that tracks and prints SwCharFormat deletions (1.91 KB, patch)
2024-08-05 12:56 UTC, Patrick (volunteer)
Details
Backtrace in lldb of deletion of SwCharFormat that causes crash (7.38 KB, text/plain)
2024-08-05 12:57 UTC, Patrick (volunteer)
Details
Backtrace in lldb where assert fails when pasting in HTML format (15.13 KB, text/plain)
2024-08-06 21:53 UTC, Patrick (volunteer)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2021-02-01 13:57:24 UTC
Description:
Crash swlo!sw::WriterMultiListener::StartListening

Steps to Reproduce:
1. Open the attached file
2. CTRL+A
3. CTRL+C
4. CTRL+N
5. CTRL+V
6. CTRL+Z
7. CTRL+SHIFT+V -> RTF

Actual Results:
Crash

Expected Results:
No crash


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.2.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 66013201749df7d5ac5ddaf377a7b3732518a93b
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: nl-NL (nl_NL); UI: en-US
Calc: CL
Comment 1 Telesto 2021-02-01 13:57:38 UTC
Created attachment 169348 [details]
Example file
Comment 2 Telesto 2021-02-01 14:00:26 UTC
Also in
Version: 7.0.0.0.beta1+ (x64)
Build ID: 2891e91a513520d68ea2b8c59c14335861a15253
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: nl-NL (nl_NL); UI: en-US
Calc: CL

no crash with
Version: 6.4.0.0.beta1+ (x64)
Build ID: 20be5cd0bdc57d812bf34a2debfe48caa51de881
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: GL; VCL: win; 
Locale: nl-NL (nl_NL); UI-Language: en-US
Calc: CL
Comment 3 Telesto 2021-02-01 14:01:13 UTC
Created attachment 169349 [details]
BT without symbols
Comment 4 Julien Nabet 2021-02-01 20:10:23 UTC
Created attachment 169364 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this.
Comment 5 Xisco Faulí 2021-02-03 16:00:52 UTC
I tried to bisect it with bibisect-linux64-6.4 but the results are not accurate.
@Telesto, please, bisect it on your side
Comment 6 Telesto 2021-02-03 18:30:02 UTC
(In reply to Xisco Faulí from comment #5)
> I tried to bisect it with bibisect-linux64-6.4 but the results are not
> accurate.
> @Telesto, please, bisect it on your side

So you could reproduce this with 6.4..while i'm looking bibisect 7.0 repro. Attempted a bibisect.. but bogus too
Comment 7 Buovjaga 2021-08-29 19:15:45 UTC
Yeah, it is somehow unreliable. It crashed for me even in the oldest of Linux 6.3 repo. Sometimes you have to attempt it as many as 5 times.

As the steps are simple, I recommend recording a macro of them, so you can run them easily without going crazy (if you want to still investigate with older repos).
Comment 8 Telesto 2022-12-24 09:16:55 UTC
No crash with macOS
Version: 7.6.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 8635c9aa8c6f1078a9e220076d5a08daf30077e8
CPU threads: 8; OS: Mac OS X 12.3.1; UI render: Skia/Metal; VCL: osx
Locale: nl-NL (nl_NL.UTF-8); UI: en-US
Calc: threaded
Comment 9 Buovjaga 2022-12-25 11:59:27 UTC
Crashed on first try both with 7.4.3 and with latest of 7.5 bibisect repo on Linux

Version: 7.5.0.1.0+ (X86_64) / LibreOffice Community
Build ID: 09848e94d20c067499ad69edf81fa80a45d0a632
CPU threads: 8; OS: Linux 6.1; UI render: default; VCL: kf5 (cairo+xcb)
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: threaded
Comment 10 Federico Castellani 2023-12-06 15:20:30 UTC Comment hidden (obsolete)
Comment 11 Buovjaga 2023-12-06 15:27:12 UTC Comment hidden (obsolete)
Comment 12 Frank Steiner 2023-12-06 15:39:41 UTC Comment hidden (obsolete)
Comment 13 Buovjaga 2023-12-06 17:00:52 UTC Comment hidden (obsolete)
Comment 14 Frank Steiner 2023-12-12 13:54:07 UTC Comment hidden (obsolete)
Comment 15 Buovjaga 2024-08-04 10:10:22 UTC
Still crashing.

Version: 25.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: ed9a557e1ceb4ffa4060024b20785f04d227e06c
CPU threads: 8; OS: Linux 6.10; UI render: default; VCL: kf6 (cairo+wayland)
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: CL threaded
Comment 16 Patrick (volunteer) 2024-08-04 22:32:27 UTC
Created attachment 195701 [details]
macOS crash log

I can reproduce this bug on macOS in my local master build. Not sure what is happening in the code though.
Comment 17 Patrick (volunteer) 2024-08-05 12:56:17 UTC
Created attachment 195717 [details]
Test patch that tracks and prints SwCharFormat deletions
Comment 18 Patrick (volunteer) 2024-08-05 12:57:58 UTC
Created attachment 195718 [details]
Backtrace in lldb of deletion of SwCharFormat that causes crash
Comment 19 Patrick (volunteer) 2024-08-05 13:13:15 UTC
Update: the crash is due to a "use after free" of an SwCharFormat instance.

In my macOS crash log in attachment #195701 [details], the crash occurs in SwClient::SwClient(SwModify*) because the SwModify* parameter is an already deleted pointer. In this particular case, the SwModify* parameter is an SwCharFormat instance stored in SwEndNoteInfo::m_pAnchorFormat.

Essentially, the SwEndNoteInfo::m_pAnchorFormat pointer is getting deleted out from underneath the SwEndNoteInfo instance. Using the debug patch in attachment #195717 [details], I was able to set a break in lldb and get a backtrace of where the SwCharFormat that matches the pointer in SwEndNoteInfo::m_pAnchorFormat gets deleted.

My lldb backtrace in attachment #195718 [details] shows the SwCharFormat instance that matched the pointer in SwEndNoteInfo::m_pAnchorFormat is deleted when undoing (step 6 in comment #0).

I am not familiar with the inner workings of the Writer code so hopefully someone might have an idea what needs to be changed to fix this bug. Does the SwCharFormat instance need to stay alive longer? Or does the SwCharFormat destructor need to somehow notify the SwEndNotInfo instance?
Comment 20 Miklos Vajna 2024-08-05 13:53:33 UTC
In SwDoc::~SwDoc(), sw/source/core/doc/docnew.cxx:523 tries to not listen to char formats anymore in the endnote config, and only sw/source/core/doc/docnew.cxx:596 deletes the char format table, so that looks like the right order in general. Is it possible that mpEndNoteInfo->EndListeningAll() should clear the endnote info's anchor format pointer, but it does not? Or something along those lines.
Comment 21 Patrick (volunteer) 2024-08-05 18:06:46 UTC
(In reply to Miklos Vajna from comment #20)
> In SwDoc::~SwDoc(), sw/source/core/doc/docnew.cxx:523 tries to not listen to
> char formats anymore in the endnote config, and only
> sw/source/core/doc/docnew.cxx:596 deletes the char format table, so that
> looks like the right order in general. Is it possible that
> mpEndNoteInfo->EndListeningAll() should clear the endnote info's anchor
> format pointer, but it does not? Or something along those lines.

AFAICT, mpEndNoteInfo->EndListeningAll() never gets called before the crash which makes sense to me as the end note should still be alive since it is held by a transferable object.

I would think that SwDoc::DelCharFormat() needs to notify SwEndNoteInfo since the SwEndNoteInfo's anchor format pointer is no longer in SwDoc after the undo but it is still needed for the transferable. Then maybe the SwEndNoteInfo can make it own copy of the anchor format instance before deletion?
Comment 22 Patrick (volunteer) 2024-08-05 22:25:34 UTC
The more I think about this, I can't help but wonder why the transferable is holding pointers from an SwDoc instance that is being edited instead of the SwDoc for the open and unmodified window that this content was originally copied from.

I understand that the content isn't rendered until pasting, but if the SwEndNoteInfo's anchor format pointer is deleted by SwDoc, doesn't that indicate that the transferable on the clipboard is now the undone content that was pasted in the second document?

Related to that, shouldn't the transferable content be an immutable snapshot instead of rendered from the SwDoc state at the time of pasting?

Maybe I just don't understand what is going on here, but it just seems odd to me that we use pointers from the currently edited document at render time instead of the data as it existed at the time of copying.
Comment 23 Patrick (volunteer) 2024-08-05 22:37:50 UTC
(In reply to Patrick Luby (volunteer) from comment #22)
> Maybe I just don't understand what is going on here, but it just seems odd
> to me that we use pointers from the currently edited document at render time
> instead of the data as it existed at the time of copying.

Just for fun, I disabled all deletion of SwDoc's SwCharFormat instances (see debug patch below) and while it stops the crash when pasting as RTF, pasting as HTML crashes.

So, there is a lot more data corruption in the transferable than I originally thought. I really think the Writer developers themselves will need to take a look at this.
Comment 24 Patrick (volunteer) 2024-08-06 21:53:38 UTC
Created attachment 195742 [details]
Backtrace in lldb where assert fails when pasting in HTML format
Comment 25 Patrick (volunteer) 2024-08-06 22:06:03 UTC
(In reply to Patrick Luby (volunteer) from comment #24)
> Created attachment 195742 [details]
> Backtrace in lldb where assert fails when pasting in HTML format

I found that the crash when pasting in HTML format is probably a separate bug: an assert fails in the HTML export code and removing the assert stops that crash.

So back to the original pasting in RTF format bug. While I am still unsure which SwDoc instance owns the SwEndNoteInfo's anchor format pointer, I think that might be able to avoid delving into the SwDoc and writerfilter code and fix the bug my just delaying the deletion of any anchor format pointers created by SwDoc.

I'll try wrapping those pointers in rtl::Reference<SwCharFormat>. If that proves infeasible, I will try moving deletion of those pointers to the ~SwDoc().
Comment 26 Patrick (volunteer) 2024-08-08 14:01:59 UTC
(In reply to Patrick Luby (volunteer) from comment #25)
> I'll try wrapping those pointers in rtl::Reference<SwCharFormat>. If that
> proves infeasible, I will try moving deletion of those pointers to the
> ~SwDoc().

I have submitted a fix for this bug in the following patch. I gave up on using shared pointers and, instead, went for the simpler approach of keeping the SwCharFormat instances alive until the SwDoc is cleared or deleted. I also replaced the assert() with a warning in the HTML export code so both pasting as RTF and HTML don't crash:

https://gerrit.libreoffice.org/c/core/+/171608

Not sure if this is a good patch or not so I'll leave it to developers more familiar with the Writer code to review and approve/reject it.
Comment 27 Commit Notification 2024-08-09 14:34:28 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/7b7347bb2112d89b2ad463a464cabe0c8926e0d0

tdf#140061 keep SwCharFormat instances alive while SwDoc is alive

It will be available in 25.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 28 Patrick (volunteer) 2024-08-09 14:39:56 UTC
I have committed a fix for this bug. The fix should be in tomorrow's (10 August 2024) nightly master builds:

https://dev-builds.libreoffice.org/daily/master/current.html

Note for macOS testers: the nightly master build installer does not overwrite any LibreOffice official versions. Instead, it will be installed as a separate application called "LibreOfficeDev" in the /Applications folder.

Because this is a "test" build, you will need to do the following steps before you launch the LibreOfficeDev application:

1. Go to the Finder and navigate to the /Applications/Utilities folder
2. Launch the "Terminal" application
3. Paste the following command in the Terminal application window and press the Return key to execute the command:

   xattr -d com.apple.quarantine /Applications/LibreOfficeDev.app
Comment 29 Commit Notification 2024-08-12 07:22:43 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

https://git.libreoffice.org/core/commit/23829f6e1cebf808f4e6ff6ed4856123ad8e8e38

tdf#140061 keep SwCharFormat instances alive while SwDoc is alive

It will be available in 24.2.6.

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 30 Commit Notification 2024-08-12 07:22:46 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/5b41e2ea38fba4403c830bf82a4095a265eb65d9

tdf#140061 keep SwCharFormat instances alive while SwDoc is alive

It will be available in 24.8.1.

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 2024-10-17 13:22:48 UTC
Samuel Adesola committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8e7e59eddedac07ea32388168bcc71f3e521a526

tdf#140061 CPPUNIT Test for fix swlo!sw::WriterMultiListener::StartListening

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