Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: Fix endless loop in process_swf5_references #9885

Merged
merged 2 commits into from
Mar 19, 2023
Merged

core: Fix endless loop in process_swf5_references #9885

merged 2 commits into from
Mar 19, 2023

Conversation

nicholascioli
Copy link
Contributor

This commit fixes an issue where ruffle locks up while trying to resolve an item with non-MovieClip parents.

While testing out the game Mystery of Time and Space (MOTAS), I encountered a bug where any room with a clock caused ruffle to completely hang when loading a different room. After de-compiling the SWF with JPEXS, I noticed that by removing the following code it would no longer hang:

myDate = new Date();
_root.uren = myDate.getHours() * 30 + myDate.getMinutes() / 2;
setProperty("uren", _rotation, _root.uren);
_root.minuten = myDate.getMinutes() * 6 + myDate.getSeconds() / 10;
setProperty("minuten", _rotation, _root.minuten);
_root.seconden = myDate.getSeconds() * 6;
setProperty("seconden", _rotation, _root.seconden);
_root.minuten = myDate.getMinutes() * 6 + myDate.getSeconds() / 10;

After loading the game in lldb, I saw that it infintely looped on process_swf5_references due to the wall clock not having any MovieClip parents up its chain, which I assume is caused by the following code which gets executed when changing rooms:

on(release){
   set("_root/:text","Let\'s see...");
   tellTarget("_root/pointer")
   {
      gotoAndStop("off");
   }
   set("_root/back/:room",room);
   tellTarget("_root/back")
   {
      gotoAndStop(eval("_root/back/:room"));
   }
   play();
}

After adding this change, the game no longer hangs.

Testing

The files and command I used for testing are as follows:

  • Command: ruffle_desktop mysteryoftimeandspace.swf -P chat=chat -P sublevel=mystery2.swf -P menu=false
  • Files: motas.zip

Once you have the game running, do the following:

  • Click on Continue

  • Click on the left-most door

  • Click on the opening of the left-most door

@Dinnerbone
Copy link
Contributor

Welcome and thank you for contributing!

This looks good to me. Would you be able to create a regression test for this?

@n0samu
Copy link
Member

n0samu commented Mar 6, 2023

Thanks so much for looking into this! The endless loop is a recent regression caused by #9447. Hopefully we can get some feedback from @CUB3D on this.

@n0samu n0samu requested a review from CUB3D March 6, 2023 23:45
@nicholascioli
Copy link
Contributor Author

Welcome and thank you for contributing!

This looks good to me. Would you be able to create a regression test for this?

Sure! I've not programmed in ActionScript before, but I'll see if I can put together a regression test.

@Dinnerbone
Copy link
Contributor

If you need help or would like someone else to try, just ask!

Copy link
Contributor

@CUB3D CUB3D left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see a test added for this, otherwise LGTM

core/src/avm1/object_reference.rs Outdated Show resolved Hide resolved
@nicholascioli
Copy link
Contributor Author

I've added a regression test and verified that it fails when run on the commit right before mine. As a note, I don't have adobe animate so I didn't have a way to verify that the FLA file is valid / non-giberish. I mostly just modified the original SWF file that caused the bug to only include the bare minimum and then used JPEXS to export the FLA project.

@n0samu
Copy link
Member

n0samu commented Mar 12, 2023

After some experiments in JPEXS, here is an even more minimal SWF:
9885_test.zip
The critical piece is that the clock sprite is placed and then immediately removed by a RemoveObject tag. This causes accessing the clock's subsprites through _root to crash Ruffle, since the clock has already been removed. It seems like in Flash Player, it just silently fails without even producing an error message in the flashlog.

Also, the FLA you exported from JPEXS doesn't produce an SWF that reproduces the issue. I don't think it's actually possible to create a FLA that reproduces the problem in this way, because the Flash authoring software does not produce frames that place and immediately remove an object. And the error does not occur in Ruffle if the object is not placed at all, or is placed and removed on different frames. Does the original game produce the error this way? I imagine that it must be removing the object some other way, not with a RemoveObject tag.

@nicholascioli
Copy link
Contributor Author

After some experiments in JPEXS, here is an even more minimal SWF: 9885_test.zip The critical piece is that the clock sprite is placed and then immediately removed by a RemoveObject tag. This causes accessing the clock's subsprites through _root to crash Ruffle, since the clock has already been removed. It seems like in Flash Player, it just silently fails without even producing an error message in the flashlog.

Also, the FLA you exported from JPEXS doesn't produce an SWF that reproduces the issue. I don't think it's actually possible to create a FLA that reproduces the problem in this way, because the Flash authoring software does not produce frames that place and immediately remove an object. And the error does not occur in Ruffle if the object is not placed at all, or is placed and removed on different frames. Does the original game produce the error this way? I imagine that it must be removing the object some other way, not with a RemoveObject tag.

Hmm, I'm not really sure as I've not used ActionScript before. The code I pasted in the first comment of this chain (including the zip) contains what I believe is the offending code. It does seem, however, that the original author of this game used paused frames to represent different rooms in each level, so I'm not sure.

Also, I see that the PR was approved. What are the next steps? Let me know if I need to update anything else!

@Herschel Herschel self-assigned this Mar 19, 2023
@Herschel
Copy link
Member

Herschel commented Mar 19, 2023

I pushed a minimal test case reproing the issue; the crux is code executing inside a button's children after the button has been removed. Placing+removing on the same frame isn't important.

There's still some issues here (tracing _root ends up incorrectly printing _level1), but fixing the hang is important for now. Thank you!

@Herschel Herschel removed their assignment Mar 19, 2023
nicholascioli and others added 2 commits March 18, 2023 19:14
This commit fixes an issue where ruffle locks up while trying to
resolve an item with non-MovieClip parents.
A simpliied test case to test the #9885, where a hang occurred when
a button is removed while executing code in a v5 SWF.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants