HarbourMasters / Shipwright

3.31k stars 495 forks source link

Concurrency Fix #4318

Closed Malkierian closed 2 months ago

Malkierian commented 3 months ago

Removed all CVarLoad uses from code to prevent thread concurrency issues. Also added mutext locking to prevent concurrent save and load operations on the save file.

Build Artifacts

Pepper0ni commented 2 months ago

Is there anything that needs testing for this aside from building seeds until I'm sure cvar crash is gone?

Malkierian commented 2 months ago

Not that I can think of. To be fair, I think I need to change the exit and reload locking to utilize the mutex as well before merging, but it's really hard to test either of those things if you're not on a console. The CVar thing was the only one that was very noticeable on PC.

Malkierian commented 2 months ago

And, for the record, after checking, utilizing the mutex in restart and exit shouldn't be necessary. I tried timing both in debug and couldn't get a single crash or corrupted save file as the system stands now.

Malkierian commented 2 months ago

I don't know what the exit routines are like for Wii U and Switch closing the app from their menus. It could be a matter of those processes just killing the apps, in which case we might want to add somewhere to quit via the SoH menu instead to ensure save integrity.