Bug 117797 - Crash / assert when resaving a specific PPT
Summary: Crash / assert when resaving a specific PPT
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
6.0.0.3 release
Hardware: All Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.2.0 target:6.1.1
Keywords: bibisected, bisected, regression
Depends on:
Blocks: imageHandling-regressions
  Show dependency treegraph
 
Reported: 2018-05-25 11:37 UTC by Aron Budea
Modified: 2018-09-03 17:51 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Budea 2018-05-25 11:37:50 UTC
Open attachment 140789 [details] (from bug 116539), and save it again as a PPT.

=> Save crashes (release build) or triggers the following assertion failure (debug build):

https://opengrok.libreoffice.org/xref/core/vcl/source/app/scheduler.cxx#597
assert(nullptr == mpSchedulerData || utl::ConfigManager::IsFuzzing());

Observed using 6.2 master daily build & 6.0.0.3 / Windows 7. Since bug 116539 mentions no crash in Linux, I'm assuming this is Windows-specific.
No crash with 5.4.0.3.
=> regression

In 6.0.0.3 and onwards there's also a wide white rectangle covering part of the slide. Might be related to the crash.

Crash report:
https://crashreport.libreoffice.org/stats/crash_details/1c4233c0-626d-4e6d-9aea-53a87f100f88
Comment 1 Aron Budea 2018-05-25 12:31:32 UTC
Interestingly the debugger breaks elsewhere:

setOriginURL(mpSwapFile->maOriginURL);
https://opengrok.libreoffice.org/xref/core/vcl/source/gdi/impgraph.cxx#1579

Here mpSwapFile->maOriginURL is ??? (undefined I assume), even though it was empty until the bRet = ImplSwapIn( xIStm.get() ) call a bit above.

Actually, ImplSwapIn( SvStream* xIStm ) calls ImplClear(), which does this:
mpSwapFile.reset();

So if the swap file is thrown away, why is its maOriginURL taken later?

Miklos, can you please take a look at what's supposed to be going on there?
Comment 2 MM 2018-05-25 13:03:36 UTC
Confirmed on windows 7 x64 with Version: 6.0.4.2 (x64)
Build ID: 9b0d9b32d5dcda91d2f1a96dc04c645c450872bf
CPU threads: 3; OS: Windows 6.1; UI render: default

crashreport.libreoffice.org/stats/crash_details/91ef0460-5d32-472a-816a-0ed4da90d367

Unconfirmed on ubuntu 16.04 x64 with Version: 6.1.0.0.alpha1+
Build ID: 47dc3115f12ff16dc326b6edd12c46e6a6ef1843
CPU threads: 2; OS: Linux 4.4; UI render: default; VCL: gtk2; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-05-17_00:32:17
Locale: en-US (en_US.UTF-8); Calc:

Seems windows only then.
Also the white rectangle isn't there in the linux build.
But saving seems a lot slower than with v6.0.0.0
Comment 3 raal 2018-05-25 15:03:22 UTC
This seems to have begun at the below commit.
Adding Cc: to Tomaž Vajngerl; Could you possibly take a look at this one? Thanks

 0b17b35d7aff81ec5ba3790ff3544d18e5f6ae44 is the first bad commit
commit 0b17b35d7aff81ec5ba3790ff3544d18e5f6ae44
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Tue May 8 17:47:24 2018 -0700

    source sha:711c2e49dd3c51877263148267344e2eb4ca7c0d
author	Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>	2018-05-08 19:45:36 +0900
committer	Tomaž Vajngerl <quikee@gmail.com>	2018-05-09 02:03:16 +0200
commit	711c2e49dd3c51877263148267344e2eb4ca7c0d (patch)
tree	f6b676ceb8a607922bdcff0b1d70567c03d5a26f
parent	f4e7d521aab79ecbb16c0518f14c7635902e9e10 (diff)
tdf#116272 use correct property name when exporting to PPT
Comment 4 Julien Nabet 2018-05-26 05:12:19 UTC
On pc Debian x86-64 with master sources updated yesterday + enable-dbgutil, I killed LO after about 30 seconds because it was too slow.
I noticed these kinds of logs:
warn:legacy.tools:5207:5207:sfx2/source/control/bindings.cxx:793: SfxBindings::Register while status-updating
warn:legacy.tools:5207:5207:sfx2/source/control/bindings.cxx:1288: Reschedule in StateChanged => buff

warn:vcl.gdi:5207:5207:vcl/source/graphic/Manager.cxx:135: Calculated size mismatch. Variable size is '-89853776' but calculated size is '114107674'
warn:vcl.gdi:5207:5207:vcl/source/graphic/Manager.cxx:135: Calculated size mismatch. Variable size is '725992024' but calculated size is '409817674'
Comment 5 Regis Perdreau 2018-05-26 06:26:32 UTC
Work perfectly with 
Version: 6.1.0.0.alpha1+
Build ID: 1e2afc9bd3062cfba6b65b45c17a08f298014239
CPU threads: 4; OS: Linux 4.8; UI render: default; VCL: gtk3; 
Locale: fr-FR (fr_FR.UTF-8); Calc: group
Linux mint 18.2 64 bits.
Comment 6 Aron Budea 2018-05-27 04:18:05 UTC
I guess the crash report and the most recent crash are different bugs, considering one was there in 6.0, and the other was introduce during 6.1 development.

Additionally, my observations in comment 1 again refer to a different bug, only observable when debugging a Windows debug build with OpenGL disabled (maybe Linux, too, haven't checked). That was introduced in the following commit:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=075e2d8b8d98b1c6daa430e9b9a396b15ba22837
author		Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>	2018-02-13 21:26:40 +0900
committer	Tomaž Vajngerl <quikee@gmail.com>	2018-02-13 20:29:24 +0100

graphic: Remember the origin URL after swap out - swap in
Comment 7 Xisco Faulí 2018-06-05 21:42:32 UTC
I can't reproduce the crash in

Version: 6.2.0.0.alpha0+
Build ID: 48b49937fed5e50d299a94063eb325799ff672e9
CPU threads: 1; OS: Windows 6.1; UI render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2018-06-05_01:47:06
Locale: es-ES (es_ES); Calc: group threaded

@MM, @Aron, @Raal, @Julien, Could you please retry with a daily build?
Comment 8 Xisco Faulí 2018-06-05 22:02:58 UTC Comment hidden (obsolete)
Comment 9 Xisco Faulí 2018-06-05 22:03:33 UTC
Not reproduced in

Version: 6.1.0.0.beta1+
Build ID: 91d8af2c5cf4e8ec0f1ce0e532e0c896de77750b
CPU threads: 16; OS: Windows 6.3; UI render: default; 
Locale: en-GB (en_GB); Calc: group threaded

either
Comment 10 Aron Budea 2018-06-06 21:50:17 UTC
Still getting it with 565340d457f41197474a75ba1b036bdc3d569041 (from 06-03), only with default rendering.
Comment 11 Aron Budea 2018-06-11 20:32:21 UTC
Adding an if guard checking if mpSwapFile isn't empty around the following lines gets rid of the debugger break and the assertion failure:

https://opengrok.libreoffice.org/xref/core/vcl/source/gdi/impgraph.cxx#1592
======
setOriginURL(mpSwapFile->maOriginURL);
mpSwapFile.reset();
======

However, I'm not sure if that makes the code consistent, and would rather leave the change to Tomaz.
Comment 12 Commit Notification 2018-08-16 20:09:25 UTC
Tomaž Vajngerl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3cd264bb09c6c14bf9eb769d508d7015c4bdc83b

tdf#117797 guard access to mpSwapFile as it may not be set

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 13 Commit Notification 2018-08-22 07:24:02 UTC
Tomaž Vajngerl committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=22dbd5904f4f69d98530fecdd9e4383dd088f76f&h=libreoffice-6-1

tdf#117797 guard access to mpSwapFile as it may not be set

It will be available in 6.1.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.
Comment 14 Aron Budea 2018-08-29 03:21:44 UTC
This bug report mentioned different issues I encountered after following the same steps. Let it only concern the issue Tomaž fixed, thanks for that! Closing as FIXED.

I'm not getting an assert anymore, perhaps that's gone as well, but saving still crashes after consuming a lot of memory, I will check how that behaves sometimes, and open a different bug report if needed.
Comment 15 Xisco Faulí 2018-09-03 17:51:11 UTC
> Let it only concern the issue Tomaž fixed, thanks for that!
> Closing as FIXED.

Setting to VERIFIED!