Item9777: Trying to revert a topic on trunk.foswiki.org crashes
Priority: Urgent
Current State: Closed
Released In: 1.1.0
Target Release: minor
Applies To: Engine
Component: Meta
Branches:
Assertion (Attempt to reload 12 over version 13) failed!
at /usr/home/trunk.foswiki.org/core/lib/Assert.pm line 80
Assert::ASSERT(0, 'Attempt to reload 12 over version 13') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Meta.pm line 858
Foswiki::Meta::loadVersion('Foswiki::Meta=HASH(0xe253c8)', 12) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Meta.pm line 309
Foswiki::Meta::load('Foswiki::Meta=HASH(0xe253c8)', 12) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Edit.pm line 206
Foswiki::UI::Edit::init_edit('Foswiki=HASH(0xe22788)', 'edit') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Edit.pm line 34
Foswiki::UI::Edit::edit('Foswiki=HASH(0xe22788)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Manage.pm line 523
Foswiki::UI::Manage::_action_restoreRevision('Foswiki=HASH(0xe22788)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Manage.pm line 45
Foswiki::UI::Manage::manage('Foswiki=HASH(0xe22788)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 316
Foswiki::UI::__ANON__() called at /usr/local/lib/perl5/site_perl/5.8.8/Error.pm line 415
eval {...} called at /usr/local/lib/perl5/site_perl/5.8.8/Error.pm line 407
Error::subs::try('CODE(0xd8e858)', 'HASH(0xe22468)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 435
Foswiki::UI::_execute('Foswiki::Request=HASH(0xdc11a8)', 'CODE(0xdc0d68)', 'manage', 1) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 277
Foswiki::UI::handleRequest('Foswiki::Request=HASH(0xdc11a8)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Engine/CGI.pm line 37
Foswiki::Engine::CGI::run('Foswiki::Engine::CGI=HASH(0x901fc8)') called at /home/trunk.foswiki.org/core/bin/manage line 24
at /usr/home/trunk.foswiki.org/core/lib/Assert.pm line 80
Assert::ASSERT(0, 'Attempt to reload 12 over version 13') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Meta.pm line 858
Foswiki::Meta::loadVersion('Foswiki::Meta=HASH(0xe253c8)', 12) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Meta.pm line 309
Foswiki::Meta::load('Foswiki::Meta=HASH(0xe253c8)', 12) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Edit.pm line 206
Foswiki::UI::Edit::init_edit('Foswiki=HASH(0xe22788)', 'edit') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Edit.pm line 34
Foswiki::UI::Edit::edit('Foswiki=HASH(0xe22788)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Manage.pm line 523
Foswiki::UI::Manage::_action_restoreRevision('Foswiki=HASH(0xe22788)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/Manage.pm line 45
Foswiki::UI::Manage::manage('Foswiki=HASH(0xe22788)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 316
Foswiki::UI::__ANON__() called at /usr/local/lib/perl5/site_perl/5.8.8/Error.pm line 415
eval {...} called at /usr/local/lib/perl5/site_perl/5.8.8/Error.pm line 407
Error::subs::try('CODE(0xd8e858)', 'HASH(0xe22468)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 435
Foswiki::UI::_execute('Foswiki::Request=HASH(0xdc11a8)', 'CODE(0xdc0d68)', 'manage', 1) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 277
Foswiki::UI::handleRequest('Foswiki::Request=HASH(0xdc11a8)') called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Engine/CGI.pm line 37
Foswiki::Engine::CGI::run('Foswiki::Engine::CGI=HASH(0x901fc8)') called at /home/trunk.foswiki.org/core/bin/manage line 24.
--
GeorgeClark - 01 Oct 2010
Also fails on Release 1.1 checkout. Bumping to Urgent
--
GeorgeClark - 01 Oct 2010
It does not crash when asserts are turned off and seems to work.
Is it just the assert that is silly?
--
KennethLavrsen - 01 Oct 2010
We chatted a bit on IRC yesterday and pharvey seemed to recall that this was something fairly important. Reloading of a meta for a new version causes some sort of other issues.
--
GeorgeClark - 01 Oct 2010
gac410 I think that CDot needs to review. Since revert is supposed to load a previous version - why does it assert if a previous rev load is attempted.
unless he wants that a new Meta object needs to be loaded. [02:38]
pharvey I am vaguely aware that the assert is very important
There was a change where $topicObject->reload() was changed to ->load() to remove any suggestion that $topicObject could/should be re-loaded with a different version
I don't recall why this functionality was removed, but I think it simplified something or fixed something to do with non-txt stores [02:39]
--
GeorgeClark - 01 Oct 2010
I've committed a fix to UI/Edit.pm. It resolves the crash, but I don't really understand why. This really needs review. Committed to Release 1.1 branch.
--
GeorgeClark - 01 Oct 2010
No, the assert is
not silly. Loading a new version over the top of an already loaded version is very dangerous, as it requires careful unloading of the object before the new revision is loaded. This caused a number of issues and led us to revert the reload method. It is still possible to
unload the object and then load it with a new revision; this is a much more measured process, and is what should be done in this case.
--
CrawfordCurrie - 01 Oct 2010
So what about George's fix? Is this OK? I did not sense a clear answer on that
I am releasing 1.1.0 this week-end. It is important that this fix is well tested and well reviewed.
--
KennethLavrsen - 01 Oct 2010
I've added code to unload() and finish() the object before re-issuing the load. Please confirm that I've done this correctly. Thanks.
--
GeorgeClark - 01 Oct 2010
I assume Crawford would have given more specific advise if he thought the fix wouldn't work.
My knowledge of
Foswik::Meta
isn't very deep, but I think George's fix is probably the same I would've tried. I guess it's just more expensive than the old
reload()
; but should be safe until Crawford suggests something else.
--
PaulHarvey - 02 Oct 2010
No feedback from Crawford, he must be out of town.
But wit Sven's and Paul's feedback and my own tests it does seem fixed.
Reverting to a previous version is something that happens every 2nd day so if Georges fix is not 100% efficient as it may have been, I am sure nooone will notice.
The important thing is that it is safe.
Closing this.
--
KennethLavrsen - 03 Oct 2010