Make sure CallbackStream doesn't get destroyed before the Music#163
Make sure CallbackStream doesn't get destroyed before the Music#163eXpl0it3r merged 1 commit intoSFML:masterfrom
Conversation
Music's destructor call's stop(), which in turn calls onSeek. For CSFML, this requires the CallbackStream to stay alive until that point. In C++, destructors for fields are called in reverse declaration order. We swap the order of the fields, putting the Music after the CallbackStream, so it gets destroyed before it.
|
See jeremyletang/rust-sfml#239 (comment) for context. |
|
Tomorrow I'll check other usages of CallbackStream to see if they satisfy this invariant. |
|
I checked the other occurrences of CallbackStream. They all seem good, with one possible exception. |
|
It's likely due to somewhat misleading naming. C++ API requires the stream to remain accessible until new font is loaded or is destroyed. |
|
It's a bit misleading indeed, but the file/stream is kept open in order to reload new characters/sizes from the font. @crumblingstatue do you think the FontStruct needs adjusting as well? |
I'm not sure, I'll have to look into it tomorrow |
|
Just noticed this merge and wondered if its the cause of an issue I raised for SFML.Net some time ago. Disposing a Music instance would crash with STATUS_STACK_BUFFER_OVERRUN error and it was concluded to be a bug in CSFML? |
|
Might be, can you reproduce it with and without this change? |
|
Just to cross my comments over. This has infact fixed the Music dispose bug in SFML.Net and is also reproducible on the current 2.5 Nuget version. |
4 participants
Music's destructor call's stop(), which in turn calls onSeek.
For CSFML, this requires the CallbackStream to stay alive until that point.
In C++, destructors for fields are called in reverse declaration order.
We swap the order of the fields, putting the Music after the
CallbackStream, so it gets destroyed before it.