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: unspecified
Hardware: Other All
: medium normal
Assignee: Not Assigned
QA Contact:
URL:
Whiteboard: complextest
Keywords:
Depends on:
Blocks: 35690
  Show dependency treegraph
 
Reported: 2011-03-25 07:41 UTC by Björn Michaelsen
Modified: 2013-03-06 12:00 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


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

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 (inactive) 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 (inactive) 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
[This is an automated message.]
This bug was filed before the changes to Bugzilla on 2011-10-16. Thus it
started right out as NEW without ever being explicitly confirmed. The bug is
changed to state NEEDINFO for this reason. To move this bug from NEEDINFO back
to NEW please check if the bug still persists with the 3.5.0 beta1 or beta2 prereleases.
Details on how to test the 3.5.0 beta1 can be found at:
http://wiki.documentfoundation.org/QA/BugHunting_Session_3.5.0.-1

more detail on this bulk operation: http://nabble.documentfoundation.org/RFC-Operation-Spamzilla-tp3607474p3607474.html
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 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 (inactive) 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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.