bugGnash - The GNU Flash player - Bugs: bug #19727, race condition playing copter.swf

 
 

bug #19727: race condition playing copter.swf

Submitted by:  Sandro Santilli <strk>
Submitted on:  Fri 27 Apr 2007 09:53:11 PM UTC  
 
Category: coreSeverity: 4 - Important
Release: NoneStatus: Fixed
Privacy: PublicAssigned to: Sandro Santilli <strk>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 07 Sep 2007 12:46:55 PM UTC, comment #22:

Latest action execution order changes fixed this.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Wed 01 Aug 2007 12:53:44 AM UTC, comment #21:

No, but I think this one is not related to timeline control.

"race condition" is related to action order.

"black blanks" was an regression, either related to parser or masks, I guess.

Zou Lunkai <zoulunkai>
Tue 31 Jul 2007 02:13:14 PM UTC, comment #20:

Zou, does your timeline design 4 fixes this bug ?

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 05 Jun 2007 09:17:13 AM UTC, comment #19:

I confirm the black blanks. Tested with AGG, not with other renderers. Btw, better have a separate bug item for that, as the "race condition" actually prevents the movie to be stabily played while the rendering bug not.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 05 Jun 2007 09:10:29 AM UTC, comment #18:

> (2)there are something visually wrong now.
> Can anyone confirm this?

attached a picture file for this, see cmp.jpg

(file #12953)

Zou Lunkai <zoulunkai>
Mon 28 May 2007 08:09:15 AM UTC, comment #17:

I also always see this behaviour. Might be due to reduced speed of parser... try defining REALLY_WAIT_FOR_FRAME in vm/ASHandlers.cpp
so that the WaitForFrame never fails.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Mon 28 May 2007 02:44:24 AM UTC, comment #16:

>>Sometimes it shows a "you must install the flash5 plugin..."
>>message and sometimes just works fine.


I just always got this message today(with both 2nd and 3rd timeline design). Is this expected or a regression?

Zou Lunkai <zoulunkai>
Thu 17 May 2007 08:13:48 AM UTC, comment #15:

The attached patch (construct.patch) queues the first frame actions to the global queue rather then to the local one.
It fixed the copter.swf case, but breaks a lot of existing testcases probably due to execution order or assumptions in the test runners.

The whole "construct" thing is bogus IMHO, and should be cleaned up. Maybe better to settle the "Timeline control" part first though, that one also needs cleanups after drafting.

(file #12785)

Sandro Santilli <strk>
Project MemberIn charge of this item.
Wed 16 May 2007 07:08:22 AM UTC, comment #14:

goto_frame queues actions resulting by execution of action tags in target frame. See line 2920 of sprite_instance.cpp.

I guess what's happening is that the queued actions won't be executed due to the fact that the "target" element of the ExecutableCode object is unloaded at time of execution.
See line 68 of ExecutableCode.h.

I think always executing queued actions will have its own unwanted side-effects as well....
Executing actions immediately, instead of queuing, likely has its side-effects too, if I'm not wrong order of execution should be:

- old child
- parent
- old child's gotoFrame

Sandro Santilli <strk>
Project MemberIn charge of this item.
Wed 16 May 2007 05:55:29 AM UTC, comment #13:

> That test is a bit complex for my taste, could you explain
> briefly what is going wrong with it?


I think I got what's wrong with it now.
Briefly, executing "gotoFrame" might change the current DisplayList, eg. adding new characters to it. If "gotoFrame" is executed in "processActionQueue()", which is the last step in "movie_root::advance(float delta_time)", then 1st frame actions in newly added characters may have no chance to get executed or will get executed in next advance. That's not correct.

To fix this, we need to find out the newly added characters in "processActionQueue()" and excuted the actions of them immediately.

Zou Lunkai <zoulunkai>
Tue 15 May 2007 07:22:36 AM UTC, comment #12:

If that assertion fails it means we messed up order of the displaylist, which should be supposedly ordered by depth.
The easiest way to catch when this happened would be encoding
that test into a testInvariant() function and call it at the end of any mutator and at the start of any inspector function.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 15 May 2007 05:57:27 AM UTC, comment #11:

(1)got an assertion in dlist.cpp, line 476. What that means?
(2)there are something visually wrong now.
Can anyone confirm this?

Zou Lunkai <zoulunkai>
Wed 09 May 2007 01:00:07 AM UTC, comment #10:

all frame numbers are 1-based and reference the _root.

expected order(or behaviour) is like this(player version higher than 4):
(1) execute actions in 2nd frame, since "check"!=1, then do nothing;
(2) set "check" to 1, jump back to the second frame;
(3) execute actions in 2nd frame, since "check"==1 now, jump to the 4th frame;
(4) "mc1" is placed in the 4th frame, frame1 actions of mc1 get executed, which is the "_root.gotoAndStop(6);"
(5) As "mc1" is removed in the 5th frmae, and "mc2" is removed in the 6th frame, the 6th frame of _root should be visually empty.

expected behaviour of player4:
(1)(2)(3) are the same as the above;
(4) "mc1" is placed in the 4th frame, frame1 actions get executed but has no effect(player4 does not support mc.gotoAndStop). _root still advances to the 5th frame.
(5) "mc1" is removed in the 5th frame. mc2 is placed in the 5th frame and frame1 actions of mc2 get executed. Then the message 'your player version is lower than 5' is printed on the screen.

Zou Lunkai <zoulunkai>
Tue 08 May 2007 11:11:04 AM UTC, comment #9:

Comments for the expected behaviour is what I was looking for.
What goes wrong should ideally be derived from taht info:

Ie: should happen A,B,C
if B doesn't happen we know what's going wrong.

"why" it is going wrong is a separate issue :)

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 08 May 2007 10:47:43 AM UTC, comment #8:

I know what the testcase want to do, or what is the expected behaviour, but I don't know why Gnash fails on it internally. I haven't tracked it. If you just have quesions about what does the testcase want to do(eg. the expected execution order), I can add some comments there. But if it is related to current gnash implementation, I donn't know now.

Zou Lunkai <zoulunkai>
Tue 01 May 2007 05:25:28 PM UTC, comment #7:

That test is a bit complex for my taste, could you explain briefly what is going wrong with it?

Sandro Santilli <strk>
Project MemberIn charge of this item.
Mon 30 Apr 2007 05:44:23 AM UTC, comment #6:

Done, see test8.

Zou Lunkai <zoulunkai>
Sun 29 Apr 2007 09:20:21 AM UTC, comment #5:

But we know there's problem there:) I'll try to make a testcase for this.

Zou Lunkai <zoulunkai>
Sun 29 Apr 2007 06:20:34 AM UTC, comment #4:

Alright my analisys was wrong. I still dunno what the problem is.
What I can tell is that the gotoAndPlay() method is NOT called
in the bogus run. THat method is in first and only frame of instance1, which is placed in frame 2 and removed in frame 3 of the root sprite.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Sun 29 Apr 2007 05:30:00 AM UTC, comment #3:

Mm..., I don't see what is the problem

> but since actions in the target frame are pushed to the global
> action queue for later execution, and they end with a stop()
> the Play() has no effect !!


sprite_instance::goto_frame() just sets the current state to "STOP" and pushes the target frame actions. What do you mean by "they end with a stop()"?

ActionScript: gotoAndPlay() will be compiled as:
ActionGotoFrame
ActionPlay
yes, ActionPlay will be executed before the target frame actions, but that's what I'd expect. And it is correct AFAIK.

Zou Lunkai <zoulunkai>
Sat 28 Apr 2007 07:39:24 PM UTC, comment #2:

I belive the problem is with the "play" part of gotoAndPlay().
This is implemented by setting playstate to Play, but since
actions in the target frame are pushed to the global action queue for later execution, and they end with a stop() the Play() has no effect !!

In order to fix this we should push a 'play' action after the call to goto_frame.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Sat 28 Apr 2007 07:28:18 PM UTC, comment #1:

Turns out this is not related to threads.
The bug can be replicated by disabling loading in a separate thread and pretending the first WaitForFrame fails and the second succeeds. This can be done with the attached patch.

The bug seems related to gotoAndPlay() !

(file #12617)

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 27 Apr 2007 09:53:11 PM UTC, original submission:

http://elegans.imbb.forth.gr/games/copter.swf

This game plays fine when disabling threaded loading, but reveals a race condition if threading is enabled.

Sometimes it shows a "you must install the flash5 plugin..." message and sometimes just works fine.

Sandro Santilli <strk>
Project MemberIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #12953:  cmp.JPG added by zoulunkai (27KiB - image/jpeg)
file #12785:  construct.patch added by strk (897B - text/x-patch)
file #12617:  COPTER_BUG_ISSUE.diff added by strk (2KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by zoulunkai (Posted a comment)
  • -unavailable- added by strk (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 24 Sep 2007 03:24:02 PM UTCstrkStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Fri 07 Sep 2007 12:46:55 PM UTCstrkStatusNone=>Ready For Test
    Tue 05 Jun 2007 09:10:29 AM UTCzoulunkaiAttached File-=>Added cmp.JPG, #12953
    Thu 17 May 2007 08:13:48 AM UTCstrkAttached File-=>Added construct.patch, #12785
    Sun 29 Apr 2007 06:20:34 AM UTCstrkSummarybug in gotoAndPlay() [ and most likely gotoAndStop() too ]=>race condition playing copter.swf
    Sat 28 Apr 2007 07:41:14 PM UTCstrkSummaryrace condition playing copter.swf=>bug in gotoAndPlay() [ and most likely gotoAndStop() too ]
    Sat 28 Apr 2007 07:28:18 PM UTCstrkAttached File-=>Added COPTER_BUG_ISSUE.diff, #12617
      Carbon-Copy-=>Added zoulunkai

    Back to the top


    Powered by Savane 3.1-cleanup1