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

UE 5.3, Most Recent Update, Any GC causes a freeze, just this once I caught a crash as well confirming my sus its the plugin. #56

Closed
nonlin opened this issue Jul 31, 2024 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@nonlin
Copy link

nonlin commented Jul 31, 2024

Using the code from the Unreal Marketplace, saw an update and did it and now I got this strange behavior.

I think prior to the update it was ok, might look at fetching and using older code from here.

Basically it seems like once I start using a coroutine and do "WaitForSeconds" once Unreal decides to do Garbage Collection it would freeze the editor. Somehow I finally got Rider to report a crash (many tests not even Rider would catch a crash, editor would just be frozen and windows didn't even think it stopped responding, but wouldn't even close).

image

@zompi2 zompi2 added the bug Something isn't working label Jul 31, 2024
@zompi2
Copy link
Owner

zompi2 commented Jul 31, 2024

Thanks for reporting, I will look into it as soon as I can.

@zompi2 zompi2 self-assigned this Aug 1, 2024
@zompi2
Copy link
Owner

zompi2 commented Aug 1, 2024

Sadly, I'm not able to reproduce neither the freeze nor the crash.

I was trying to just run WaitForSeconds coroutine and forcing GC and I was trying to run WaitForSeconds and destroy the calling Actor before the time is up, forcing GC on it, but still the ECF handles everything gracefully.

I found a small issue, probably not related to this problem, but I've pushed some fixes to the version 3.3.9. They are already on github, UE Marketplace will take few hours to process it. I have no faith that this is it, but maybe...

Otherwise I'd need an example of how you use WaitForSeconds and maybe then I will be able to reproduce the issue. It might be because of some more complicated flow I'm not able to imagine.

@nonlin
Copy link
Author

nonlin commented Aug 1, 2024

More context, I literally only use the coroutine once in one way, this is the way.

FECFCoroutine ACustomGameMode::SpawnNotesForBar_Implementation(TArray<FNotes> Bar, float ExpectedHitTime)
{
    for (int i = 0; i < Bar.Num(); i++)
    {
        if (Bar[i].bLeft)
        {
            SpawnArrow(0, ExpectedHitTime);
        }
        if (Bar[i].bDown)
        {
            SpawnArrow(3);
        }
        if (Bar[i].bUp)
        {
            SpawnArrow(2, ExpectedHitTime);
        }
        if (Bar[i].bRight)
        {
            SpawnArrow(1, ExpectedHitTime);
        }
        co_await FFlow::WaitSeconds(this, FMath::Max((SecondsPerBeat / Bar.Num()) - this->GetWorld()->GetDeltaSeconds(), 0.0f));
    }
}

@nonlin
Copy link
Author

nonlin commented Aug 1, 2024

Also note the last comment here. Seems related but can't see me not having a UObject without the UPROEPRTY macro on it.

landelare/ue5coro#26 (comment)

@zompi2
Copy link
Owner

zompi2 commented Aug 1, 2024

Ok, I was able to reproduce it. I just had to run multiple co_awaits in one coroutine and run GC.

The problem has appeared in the update 3.3.6 where I was trying to resolve the potential problem of dangling coroutine handles, which might occur if the coroutine has never being resumed.
Unfortunatelly, explicit destroy on coroutine handle lead to broken heap allocation, which cause infinite loop or crash after Garbage Collector was called.

I reverted changes from 3.3.6 and let coroutine handles clean itself after the coroutine function. The only moment the handle will not clean itself is if it will never be resumed (for example, because the calling Actor has been destroyed before the coroutine resume). But it's an edge case and the potential memory leak will not be so big. I don't know how to handle such situations properly too.

The version on Marketplace will be live in few hours.

@zompi2
Copy link
Owner

zompi2 commented Aug 1, 2024

Just writing down to not forget: The idea of how to handle it:

  1. Add a flag to coroutine promise informing the coroutine has been finished (the best place will be in return_void)
  2. In the awaiter destructor check if it has:
    I. invalid owner
    II. valid coroutine handle
    III. coro handle has not been finished. If not - mark as finished and destroy it explicitly.
  3. The final_suspend should stay as suspend_never

@zompi2
Copy link
Owner

zompi2 commented Aug 6, 2024

The version 3.3.11 with fixed coroutine handles clearups is on the Marketplace. @nonlin - can you confirm if everything is ok, so I could close this issue?

@zompi2
Copy link
Owner

zompi2 commented Oct 1, 2024

I did not receive any feedback, I assume everything works fine. I'm closing the issue.

@zompi2 zompi2 closed this as completed Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants