Bug 35663 - complex.sfx2.UndoManager fails
Summary: complex.sfx2.UndoManager fails
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: complextest
Keywords:
Depends on:
Blocks: Dev-subsequenttest-failures
  Show dependency treegraph
 
Reported: 2011-03-25 07:41 UTC by Björn Michaelsen
Modified: 2023-06-25 03:13 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
complex.sfx2.UndoManager failure (3.42 KB, text/plain)
2011-03-25 07:44 UTC, Björn Michaelsen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Michaelsen 2011-03-25 07:41:01 UTC
The test complex.sfx2.UndoManager in sfx2 fails.
To reproduce, reenable the test in sfx2/JunitTest_sfx2_complex.mk and run:
 cd smoketestoo_native && build --all
 cd sw && make -sr subsequentcheck
Comment 1 Björn Michaelsen 2011-03-25 07:44:43 UTC
Created attachment 44849 [details]
complex.sfx2.UndoManager failure
Comment 2 Björn Michaelsen 2011-03-26 04:06:56 UTC
Make all bugs with whiteboard status "unoapitest" and "complextest" block the 35690 subsequenttests metabug.
Comment 3 Björn Michaelsen 2011-03-26 04:10:15 UTC
All subsequenttest bugs now block their own subsequenttests metabug:

 https://bugs.freedesktop.org/show_bug.cgi?id=35690

=> Removing 35657, 35660, 35661, 35661, 35662, 35663, 35666, 35667 as blockers from 35673

35668 stays as it is a reproducable crasher.
Comment 4 Björn Michaelsen 2011-05-23 03:11:12 UTC
I added another assert:
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=1151eaf2612c0037d7bbdd5c45a54b0346377f20

I seems like unlike in other app performing the simple operation unlocks the UndoManager.

@kohei: Could you have a look?
Comment 5 Kohei Yoshida 2011-05-23 06:45:11 UTC
(In reply to comment #4)

> @kohei: Could you have a look?

A look at what exactly?  I didn't modify UndoManager.

Also, what does this test failure mean?
Comment 6 Björn Michaelsen 2011-05-23 06:56:43 UTC
The asserting at:

http://cgit.freedesktop.org/libreoffice/libs-core/tree/sfx2/qa/complex/sfx2/UndoManager.java?id=1151eaf2612c0037d7bbdd5c45a54b0346377f20#n784

fails for Calc and only for Calc. It is fine for other apps. The code locks the undo manager, performs some basic action(*), and wants to unlock the undo manager, which fails because the undo manager was unlocked as a side effect of the operation already. So changing a value in a cell in calc has weird and unexpected effects on the undo manager.


(*) for Calc: http://cgit.freedesktop.org/libreoffice/libs-core/tree/sfx2/qa/complex/sfx2/undo/CalcDocumentTest.java?id=1151eaf2612c0037d7bbdd5c45a54b0346377f20#n43
writing a different value in A1
Comment 7 Kohei Yoshida 2011-05-23 07:05:48 UTC
Ah, yes.  I remember this.  I did that because the Calc code by and large was not ready for this ref-counted locking of the new undo manager, and to change that means substantial change in how Calc handles undo operations in general.  Without that, undo becomes disabled at all times plus causes crash.

Looks like when this undo manager change was made in OOo, Calc's code was not really adopted fully and we are paying the price for that now. *sigh*

I also don't know exactly what the new undo manager expects to do.  Why does it have to do refcounted locking anyway?
Comment 8 Björn Michaelsen 2011-05-23 14:40:47 UTC
see: http://comments.gmane.org/gmane.comp.openoffice.devel.api/21128
for the whole story
Comment 9 Björn Michaelsen 2011-12-23 11:48:19 UTC Comment hidden (obsolete)
Comment 10 Not Assigned 2012-07-09 15:39:10 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=54fbcc39a794025fb419addcd86080c7db308235

fdo#35663: enable UndoManager test, but disable failing Calc test
Comment 11 Michael Stahl (allotropia) 2012-07-09 15:41:23 UTC
ah there is already a bug about this; so indeed only the Calc part
of the UndoManager test fails.

note that last week some commits from CWS fs34b were merged that
changed the SfxUndoManager to no longer count the DisableUndo calls
but instead maintain a simple boolean flag.

Kohei, could you please check if that changes anything for
Calc, and if you can make the test work again?
Comment 12 Kohei Yoshida 2012-07-10 13:21:16 UTC
(In reply to comment #11)
> ah there is already a bug about this; so indeed only the Calc part
> of the UndoManager test fails.
> 
> note that last week some commits from CWS fs34b were merged that
> changed the SfxUndoManager to no longer count the DisableUndo calls
> but instead maintain a simple boolean flag.
> 
> Kohei, could you please check if that changes anything for
> Calc, and if you can make the test work again?

I re-enabled the test and ran make check, but I still get this:

finished class: complex.sfx2.UndoManager
--------------------------------------------------------------------------------

Time: 14.171
There was 1 failure:
1) checkCalcUndo(complex.sfx2.UndoManager)
java.lang.AssertionError: Undo manager gets unlocked as a side effect of performing a simple operation
	at complex.sfx2.UndoManager.impl_testLocking(UndoManager.java:774)
	at complex.sfx2.UndoManager.impl_checkUndo(UndoManager.java:630)
	at complex.sfx2.UndoManager.checkCalcUndo(UndoManager.java:125)

FAILURES!!!
Tests run: 12,  Failures: 1

So, I guess the coast is not yet clear.
Comment 13 Björn Michaelsen 2013-03-06 12:00:55 UTC
Bug is still very much alive, I see fails in checkBrokenScripts and checkSerialization on builders -- so doesnt seem to be confined to calc as:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=54fbcc39a794025fb419addcd86080c7db308235

suggests.
Comment 14 QA Administrators 2015-02-19 15:36:40 UTC Comment hidden (obsolete)
Comment 15 QA Administrators 2016-02-21 08:34:35 UTC Comment hidden (obsolete)
Comment 16 Michael Stahl (allotropia) 2016-02-22 21:05:53 UTC
still fails on master:

java.lang.AssertionError: Undo manager gets unlocked as a side effect of performing a simple operation
Comment 17 QA Administrators 2017-03-06 14:56:24 UTC Comment hidden (obsolete)
Comment 18 Julien Nabet 2018-06-23 21:50:07 UTC
Just for the record, on pc Debian x86-64 with master sources updated today, I could still reproduce this.
Comment 19 QA Administrators 2019-06-24 02:44:46 UTC Comment hidden (obsolete)
Comment 20 QA Administrators 2021-06-24 03:46:48 UTC Comment hidden (obsolete)
Comment 21 QA Administrators 2023-06-25 03:13:30 UTC
Dear Björn Michaelsen,

To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year.

There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present.

If you have time, please do the following:

Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/

If the bug is present, please leave a comment that includes the information from Help - About LibreOffice.
 
If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice.

Please DO NOT

Update the version field
Reply via email (please reply directly on the bug tracker)
Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not 
appropriate in this case)


If you want to do more to help you can test to see if your issue is a REGRESSION. To do so:
1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from https://downloadarchive.documentfoundation.org/libreoffice/old/

2. Test your bug
3. Leave a comment with your results.
4a. If the bug was present with 3.3 - set version to 'inherited from OOo';
4b. If the bug was not present in 3.3 - add 'regression' to keyword


Feel free to come ask questions or to say hello in our QA chat: https://web.libera.chat/?settings=#libreoffice-qa

Thank you for helping us make LibreOffice even better for everyone!

Warm Regards,
QA Team

MassPing-UntouchedBug