Item9777: Trying to revert a topic on trunk.foswiki.org crashes

pencil
Priority: Urgent
Current State: Closed
Released In: 1.1.0
Target Release: minor
Applies To: Engine
Component: Meta
Branches:
Reported By: GeorgeClark
Waiting For: Main.CrawfordCurrie
Last Change By: KennethLavrsen
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
 

ItemTemplate edit

Summary Trying to revert a topic on trunk.foswiki.org crashes
ReportedBy GeorgeClark
Codebase 1.1.0 beta1, trunk
SVN Range
AppliesTo Engine
Component Meta
Priority Urgent
CurrentState Closed
WaitingFor CrawfordCurrie
Checkins distro:8883e1dd2ae2 distro:b1d276be6dad distro:4dbbc0db3792 distro:354fa455205f
TargetRelease minor
ReleasedIn 1.1.0
Topic revision: r13 - 03 Oct 2010, KennethLavrsen
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy