ppy / osu-framework

A game framework written with osu! in mind.
MIT License
1.66k stars 417 forks source link

Crashes on Android after app is resumed if running in multi-threaded mode #4994

Open bdach opened 2 years ago

bdach commented 2 years ago

As reported in https://github.com/ppy/osu/discussions/16512 Follow-up to #4200

Looks like the context preservation added with https://github.com/ppy/osuTK/pull/74 only works on some devices because android gonna android (which I half-expected because the android documentation seemed to indicate that this may be the case), so it looks like in the worst case scenario there is still a need for the game to be fully cycled to continue working after resume.

bdach commented 2 years ago

So seeing that this looks to be pretty much the number 1 issue per user/event count on sentry I went back to this one today to try and see if I can fix, but I'm having two problems:

ppy-sentryintegration[bot] commented 2 years ago

Sentry issue: OSU-1E

ppy-sentryintegration[bot] commented 2 years ago

Sentry issue: OSU-4

bdach commented 2 years ago

Upon further investigation reproduction requires multithreaded execution. May not be the context being lost after all, rather than it being accessed before the game is allowed to do so.

bdach commented 2 years ago

Looked into this some more today but I don't have much to show for it. I am locally seeing 2 disparate errors: a BAD_ACCESS one and a BAD_SURFACE one.

The BAD_ACCESS one seems to be caused by multiple threads trying to make the gl context current. Removing this MakeCurrent() call on the osutk side appears to fix it.

I don't know what causes the BAD_SURFACE one and I don't know how to fix it. Documentation is scarce to say the least, and I don't know most of osutk code. If suppressing the errors is considered urgent, then all I can do in the short term is to go back to killing the app if it is exited in multithreaded mode.

peppy commented 2 years ago

That MakeCurrent() call definitely looks suspicious (and likely unnecessary).

Does changing that alone make crashing less likely?

bdach commented 2 years ago

No, it only changes error 1 to error 2. It's still a 100% crash either way.

Going to poke some more but my hopes are low.

smoogipoo commented 2 years ago

As an aside, is there any way we can use SDL for Android?

bdach commented 2 years ago

Had that thought too, then saw these two pages:

https://wiki.libsdl.org/Android https://github.com/libsdl-org/SDL/blob/main/docs/README-android.md

and slowly backed away. A lot of eldritch incantations and corralling jni and what have you. I have zero clue about any of it.

bdach commented 2 years ago

Tentatively closing since the two pulls link above in conjunction fix all failures I could reproduce locally, so hopefully this is gone next release. If it's not, I'll reopen.

bdach commented 2 years ago

This came up again (https://github.com/ppy/osu/issues/20972) - reopening.

VocalFan commented 2 years ago

Ah, no wonder I didn't see this. It was closed.

VocalFan commented 2 years ago

A paste of my issue for this:

Type

Crash to desktop

Bug description

Sometimes, I like to have osu! play in the background so I can listen to music using all the beatmaps I have downloaded! However, I've noticed that sometimes, when I manually lock my phone, or my phone autolocks, I'll hear a quick noise from the game before it seems to have closed when I unlock my phone. (Sounds like the Bomb/Fatal Error sound?) Requiring me to reopen the game.

Screenshots or videos

No response

Version

2022.1022.0-lazer

Logs

runtime.log performance.log network.log database.log