ScoreSaber / scoresaber-plugin

The PC plugin for ScoreSaber
MIT License
61 stars 21 forks source link

Race condition in Replay serialization #13

Closed Qwasyx closed 11 months ago

Qwasyx commented 1 year ago

There seems to be a race condition in ReplayFileWriter.Write. Potentially we need to make the Export functions return actual copies of the data (or even more likely do a swap return) to prevent this from happening.

In general we should check if more things are prone to race conditions in this system.

[DEBUG @ 16:27:36 | ScoreSaber] Packaging score...
[DEBUG @ 16:27:36 | ScoreSaber] Checking leaderboard ranked status...
[DEBUG @ 16:27:36 | ScoreSaber] Packaging replay...
[DEBUG @ 16:27:36 | ScoreSaber] Writing replay with id: e443539d-2c4a-437d-8821-4d3350038956
[DEBUG @ 16:27:36 | ScoreSaber] Failed to write replay: System.ArgumentException: Destination array was not long enough. Check destIndex and length, and the array's lower bounds
[DEBUG @ 16:27:36 | ScoreSaber]   at System.Array.Copy (System.Array sourceArray, System.Int32 sourceIndex, System.Array destinationArray, System.Int32 destinationIndex, System.Int32 length) [0x000da] in <eae584ce26bc40229c1b1aa476bfa589>:0 
[DEBUG @ 16:27:36 | ScoreSaber]   at System.Collections.Generic.List`1[T].CopyTo (T[] array, System.Int32 arrayIndex) [0x00000] in <eae584ce26bc40229c1b1aa476bfa589>:0 
[DEBUG @ 16:27:36 | ScoreSaber]   at System.Collections.Generic.List`1[T]..ctor (System.Collections.Generic.IEnumerable`1[T] collection) [0x0003b] in <eae584ce26bc40229c1b1aa476bfa589>:0 
[DEBUG @ 16:27:36 | ScoreSaber]   at ScoreSaber.Core.ReplaySystem.Data.ReplayFileWriter.Write (ScoreSaber.Core.ReplaySystem.Data.ReplayFile file) [0x00065] in <de5bb7d7647e4894a6fcfa7716ace5e0>:0 
[DEBUG @ 16:27:36 | ScoreSaber] Replay written: e443539d-2c4a-437d-8821-4d3350038956
[DEBUG @ 16:27:36 | ScoreSaber] Failed to upload (failed to serialize replay)
[DEBUG @ 16:27:36 | ScoreSaber] Failed to upload score.
Qwasyx commented 1 year ago

Update on this: It seems to primarily be the PoseRecorder that keeps recording throughout the serialization caused by Zenject not deconstructing the object in time. We probably should simply disable it once Export() is called on it.