We tested this on Win7 / Win10 / Kubuntu 17.10, with LibO 5.4.6.2 and 6.0.3.2 both with Calc and Writer. Clicking the Print Preview on a signed (odt / ods) crashes LibO. Removing the signature and then clicking Print Preview no crash. May I note that as long time LibO users (starting from OO2.4) we a getting a bit worried about changes being made in areas of code that have worked well for a very long period, apparently without sufficient testing? Problems with signing documents started to appear around 5.1 or so.
I can reproduce it in Version: 6.1.0.0.alpha0+ Build ID: b36ea44dcbdb862b0ac6e6cdaf27225b43dc0c7e CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); Calc: group
Created attachment 141397 [details] gdb backtrace
Regression introduced by: author Katarina Behrens <Katarina.Behrens@cib.de> 2017-05-31 18:26:07 +0200 committer Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de> 2017-06-08 14:44:25 +0200 commit 2e11bbd288ec59d0ff3368cc5c5ef83e2f5133a2 (patch) tree 9115b59dd5c354a8bf3148a59a01707e61780024 parent 0632208977a204195a4f5b9e727aed511ece075f (diff) tdf#105566: Account for 'signature used to be ok but isn't anymore' Bisected with: bibisect-linux64-6.0 Adding Cc: to Katarina Behrens For the bisection, I used attachment 54849 [details] from bug 44182
@Xisco, that was lightning fast, thanks!
> May I note that as long time LibO users (starting from OO2.4) we a getting a > bit worried about changes being made in areas of code that have worked well > for a very long period, apparently without sufficient testing? Dude, I totally understand that the fact software has bugs is sometimes frustrating AF. But saying that something has been implemented "apparently without sufficient testing" and not having much of a clue how much testing is really happening [1] is not going to win you many friends. It's also not going to motivate people to swiftly deal with your bugreports although it might be tempting to believe otherwise. No amount of testing ever is going to find all bugs and we all have to live with it. [1] unit tests, API tests, UI tests, continuous integration running those tests 50+ times a day, crashtesting, fuzzing, army of QA people testing dailies, dedicated QA person paid by TDF, peer code review.
Created attachment 141432 [details] bt part On pc Debian x86-64 with master sources updated today, I could reproduce this. It seems a kind of infinite recursive loop. I attached bt part showing it between sfx2/source/doc/objserv.cxx:1066 et sfx2/source/doc/objserv.cxx:1062
@Katerina I know how much testing is going on. But LibO is a big product, big testing is necessary. I don't want to offend any one, I think all bugs happen "apparently without sufficient testing". The question is how can we prevent these things from happening. What worries me that with each release bug reports explode. There was a time where there was a graph showing the number of open bugs... It's great that features get added and stuff gets improved. And I really appreciate all the work that goes into that. But there are people who actually *use* LibO and rely on existing functions to work. People who I convinced LibO is a better product than MS, who come to me to ask why it crashes when they click on the Print Preview button. And there seem to be areas (FileOpen dialog, e-mail merge, signing, pdf export, and with recent version even formatting of old documents that break when opening) where bugs appear at the same speed as they get fixed. Which is good and bad at the same time. (PS I don't think you want me to be your dude, I'm 51)
I wonder if a mutex could help but there a lot of types of these in LO and I don't know how to choose :-(
(In reply to Julien Nabet from comment #8) > I wonder if a mutex could help but there a lot of types of these in LO and I > don't know how to choose :-( It won't help. Indeed, I added this: +fprintf(stderr, "%p instance\n", this); in SfxObjectShell::GetState_Impl, it's the same thread
I submitted a patch to review here: https://gerrit.libreoffice.org/#/c/53183/
(In reply to Julien Nabet from comment #10) > I submitted a patch to review here: > https://gerrit.libreoffice.org/#/c/53183/ Sorry, the patch is wrong, it resulted to just disable the call to remove and add info bar :-(
Katarina: any update about this one? If not, what about reverting 2e11bbd288ec59d0ff3368cc5c5ef83e2f5133a2 for the moment?
Katarina Behrens committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=43459bac67363f49aadd851e686d4a74b8ddc256 tdf#117039: update infobar instead of removing and re-adding it It will be available in 6.1.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.
Katarina Behrens committed a patch related to this issue. It has been pushed to "libreoffice-6-0": http://cgit.freedesktop.org/libreoffice/core/commit/?id=8853067a29e091a230a6397bd7c100494ae9f5bf&h=libreoffice-6-0 tdf#117039: update infobar instead of removing and re-adding it It will be available in 6.0.5. 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.
Katarina Behrens committed a patch related to this issue. It has been pushed to "libreoffice-5-4": http://cgit.freedesktop.org/libreoffice/core/commit/?id=65936059966f92e983ac0d3553ade9ff37884a4e&h=libreoffice-5-4 tdf#117039: update infobar instead of removing and re-adding it It will be available in 5.4.8. 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.
@Katrina Thanks so much. I'll get my colleague to update his windows version. I'll test on Kubuntu. Are there any code paths (other than print preview) that we should pay particular attention too?
(In reply to Ferry Toth from comment #16) > @Katrina Thanks so much. I'll get my colleague to update his windows > version. I'll test on Kubuntu. > > Are there any code paths (other than print preview) that we should pay > particular attention too? This isn't the first time this code path ran into infinite recursion, see this commit: https://cgit.freedesktop.org/libreoffice/core/commit/?id=d8faf3bf9a82e8f49340b5020ec4ee931cc2f3f4 and its revert b/c failed unit test: https://cgit.freedesktop.org/libreoffice/core/commit/?id=b805e5705cfaafd8e54283840bec23afc789124a and a different fix (unit tests green): https://cgit.freedesktop.org/libreoffice/core/commit/?id=e744e9f4492d3013742fcdb6254cd76528870e9d So ye this is an example of how humans were able to creatively outsmart unit test that obviously exercises the given functionality :| Anything that alters the state or the visibility of info bar(s) such as: editing signed document, adding/deleting document signature, invalidating document signature, switching the document to read-only mode and back, going to print preview and back puts this code path to test.
Verified in Version: 6.1.0.0.alpha1+ Build ID: f1579d3d6c5f5f3a651825e035b93bee7a4f43c6 CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); Calc: group @Bubli, thanks for fixing this, should it be closed as RESOLVED FIXED ?
> @Bubli, thanks for fixing this, should it be closed as RESOLVED FIXED ? The issue as originally reported is certainly fixed. At least nobody complained so far
Sorry for not responding earlier. We have a holiday here, so colleagues with Windows not available. I planned to test also on Kubuntu and version 5.4, but AFAIKT those are not yet available as daily build. But I understand the urgency and tested myself in Win10 32bits in a Virtualbox. Fortunately I am very good in finding bugs. It appears the bug is partly fixed. But I can still get a crash. Here is what I tested: libo-60~2018-05-04_04.45.57_LibreOfficeDev_6.0.5.0.0_Win_x86.msi Win10 32bits (updates may be 3 months behind) - open LibO (starts in safe mode) - take unsigned document - check print preview: OK - check read only: OK - check read only + preview: OK - sign it: OK - status bar shows document signed icon: OK - title bar shows (signed): OK - info bar appears with signed status: OK - check print preview: OK - check read only: OK - check read only + preview: OK - dismiss info bar (click x in right corner) - check print preview: CRASH Do we need to reopen this? Because I don't see how we can un'Fixed' it.
Hi Ferry, Thank you very much for retesting it, I do confirm the crash when the infobar is removed, nice catch! I will create a follow-up bug as the issue was introduced in another commit, e744e9f4492d3013742fcdb6254cd76528870e9d
All I did was follow @Katerina's instructions. I looks like this info bar is causing griefs heavier than our offenses (to paraphrase Shakespeare). If I could hide it with an option I would.
Katarina Behrens committed a patch related to this issue. It has been pushed to "libreoffice-5-4-7": http://cgit.freedesktop.org/libreoffice/core/commit/?id=caf1019dd2b575e6dcfe46d05af5e444393d05ab&h=libreoffice-5-4-7 tdf#117039: update infobar instead of removing and re-adding it It will be available in 5.4.7. 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.
Zdeněk Crhonek committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/8cab1756cf17279ef9fb36e2032d4fb76dbc0070%5E%21 uitest for bug tdf#117039 It will be available in 6.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.