Bug 145377 - Crash in: SvxScriptErrorDialog::ShowDialog(SvxScriptErrorDialog *,void *)
Summary: Crash in: SvxScriptErrorDialog::ShowDialog(SvxScriptErrorDialog *,void *)
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
7.2.2.2 release
Hardware: All All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.3.0 target:7.2.3
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-29 09:23 UTC by Mike Kaganski
Modified: 2021-11-01 11:14 UTC (History)
1 user (show)

See Also:
Crash report or crash signature: ["SvxScriptErrorDialog::ShowDialog(SvxScriptErrorDialog *,void *)"]


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2021-10-29 09:23:33 UTC
This bug was filed from the crash reporting server and is br-e4babdd8-bc12-48c5-899c-e4174982fa4e.
=========================================

1. Start LO
2. Make sure Java is enabled
3. In Start Center, navigate to Tools->Macros->Organize Macros->BeanShell...
4. Expand LibreOffice Macros/HelloWorld, and select helloworld.bsh
5. Click [Edit] button
6. Close the opened BeanShell script editor
7. Repeat points 3-4
8. Click [Run] button at the bottom

=> crash

It is not 100% reproducible, and I don't know if 5-6 are necessary; however, in my testing, both times (out of ~5 attempts) that I had the crash were when I did 5 and 6.

Tested with Version: 7.2.2.2 (x64) / LibreOffice Community
Build ID: 02b2acce88a210515b4a5bb2e46cbfb63fe97d56
CPU threads: 12; OS: Windows 10.0 Build 19043; UI render: default; VCL: win
Locale: ru-RU (ru_RU); UI: en-US
Calc: threaded
Comment 1 Mike Kaganski 2021-10-29 10:34:42 UTC
Under debug build, it fails an assert.

It seems to be a use-after-free case; the error message being shown in IMPL_LINK(SvxScriptOrgDialog, ButtonHdl, weld::Button&, rButton, void) in 'catch ( reflection::InvocationTargetException& ite )' uses getDialog(), and that gets posted as pData->pParent in SvxScriptErrorDialog::ShowAsyncErrorDialog; and in IMPL_STATIC_LINK( SvxScriptErrorDialog, ShowDialog, void*, p, void ), the dialog (or some parts of it?) is already destroyed. I suppose the dialog is the BeanShell Macros dialog, that is being closed on the "Run" button press.

Caolan, could you please take a look at it? What could be done in this case?
Comment 2 Caolán McNamara 2021-10-29 11:51:15 UTC
I'm guessing this became a problem at...

commit 2b47c5f295589acb18d4404137c6b72d20f019b6
Date:   Mon Jun 8 15:45:10 2020 +0200

    remove the fake SvxScriptErrorDialog code
    
    which just forwards to an async real dialog.

Seeing as the script dialog is dismissed by the m_xDialog->response(RET_CANCEL) before the error dialogs get a chance to run, then I would imagine just changing things to give the error dialogs the parent of the script dialog rather than the script dialog to use as their parent
Comment 3 Mike Kaganski 2021-10-29 14:35:30 UTC
(In reply to Caolán McNamara from comment #2)
> Seeing as the script dialog is dismissed by the
> m_xDialog->response(RET_CANCEL) before the error dialogs get a chance to
> run, then I would imagine just changing things to give the error dialogs the
> parent of the script dialog rather than the script dialog to use as their
> parent

Independent of this specific fix: wouldn't it be more robust to pass some (pointer to) smart reference to the user event; and that would allow to find the real parent (e.g., the dialog itself if it's not disposed; otherwise, its parent, and so on in chain)?
Comment 4 Commit Notification 2021-10-30 09:07:56 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d33703e26a926507637733612ef169b3f3e05002

Resolves: tdf#145377 don't use dismissed dialog as parent for error dialogs

It will be available in 7.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 5 Caolán McNamara 2021-10-30 10:06:30 UTC
done in trunk, backport to 7-2 in gerrit
Comment 6 Commit Notification 2021-10-30 20:41:35 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/2afbef0037f022a59ed1198f1f84e454d070df92

Related: tdf#145377 call select handler if restoring previously selected script

It will be available in 7.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 7 Commit Notification 2021-11-01 11:10:35 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/68e3ed223eec7ccfe1a0a1f0cd8e09b88ca9e313

Resolves: tdf#145377 don't use dismissed dialog as parent for error dialogs

It will be available in 7.2.3.

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 8 Commit Notification 2021-11-01 11:14:48 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/149df697ec2f4db0be891becef217312ded3ab88

Related: tdf#145377 call select handler if restoring previously selected script

It will be available in 7.2.3.

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.