search

aws / lumberyard

Amazon Lumberyard is a free AAA game engine deeply integrated with AWS and Twitch – with full source.
Other
2.03k stars 540 forks source link

Update EntityContext.cpp #411

Closed CDufour909 closed 4 years ago

CDufour909 commented 5 years ago

Fixes a freeze when a slice instantiation is cancelled on a callback from another slice instantiation

This change fixes a freeze when a slice instantiation is cancelled on a callback from another slice instantiation. So the problematic case is when a behavior cancels slice instantiation while the callback from a newly ready asset is being processed. The problematic case is the following:

  1. EntityContext has queuedSliceInstantiations in which we find 2 entries: {SliceA, SliceB}
  2. SliceA is loaded and EntityContext::OnAssetReady is called with the Id of SliceA
  3. Tickbus processes the queued “instantiateCallback” and erases SliceA from the queued Instantiations (the iterator in the instantiateCallback points to SliceB at this point)
  4. While processing the SliceInstantiationResultBus::Events::OnSliceInstantiated for SliceA, a behavior down the callstack (either code or script) calls EntityContext::CancelSliceInstantiation for SliceB which is instantly removed from the queuedSliceInstantiations list (thusly invalidating the iterator in “instantiateCallback” who was pointing to SliceB)
  5. EntityContext::OnAssetReady’s instantiateCallback continue to process other callbacks but is left with an invalid iterator which is neither pointing to a valid entry nor pointing to the list’s end so it keeps looping forever

To fix this issue, I'm proposing to queue CancelSliceInstantiation to tickbus thus ensuring that it will not be processed in an OnAssetReady callback.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

AMZN-Gene commented 5 years ago

I noticed nobody replied to the pull request here, but an internal ticket has been created to investigate pulling this back into mainline. Thanks again :)

rustamserg commented 5 years ago

Priority 1 FYI we had similar issue in this file as well. There was a pull request https://github.com/aws/lumberyard/pull/318 with another version of the fix some time ago

AMZN-puvvadar commented 4 years ago

Hey all,

We found there to be quite a few complexities related to this and were unable to take it as is consequently. We'll be examining this further on our end. Thanks again for the submission!