brokenphilip / OMSIPresence

Discord Rich Presence plugin for OMSI 2
4 stars 0 forks source link

Cross-thread exception handling error/crash #2

Closed brokenphilip closed 1 year ago

brokenphilip commented 1 year ago

Whenever Rich Presence is about to be updated (Discord::Update()), a Vectored Exception Handler (VEH) is applied. VEHs are process-wide, meaning our exception handler may incorrectly catch an exception from another thread and terminate OMSI 2, when it should only catch exceptions inside of Discord::Update().

I have not stumbled upon this issue at all until earlier today, when I left the game open in the background and it suddenly closed. The final few entries of the logfile.txt are as follows:

[... same 3 lines repeatedly ...]
19537 13:47:54 -  -       Information: Try placing random bus: 
19538 13:47:54 -  -   Error:           Fehler bei Bereichsprüfung: AMUAV.CNAVO.MV.C
19539 13:47:54 -  -   Error:           You want to create vehicle  - it is invalid!
19540 13:47:54 -  -       Information: Try placing random bus: 
19541 13:47:54 -  -   Error:           Fehler bei Bereichsprüfung: AMUAV.CNAVO.MV.C
19542 13:47:54 -  -   Error:           You want to create vehicle  - it is invalid!
19543 13:47:55 -  -       Information: Try placing random bus: 
19544 13:47:55 -  -   Error:           Fehler bei Bereichsprüfung: AMUAV.CNAVO.MV.C
19545 13:47:55 -  -   Error:           You want to create vehicle  - it is invalid!
19546 13:47:55 -  -       Information: Try placing random bus: 
19547 13:47:55 -  -   Error:           Fehler bei Bereichsprüfung: AMUAV.CNAVO.MV.C
19548 13:47:55 -  -   Error:           You want to create vehicle  - it is invalid!
19549 13:47:56 -  -       Information: Try placing random bus: 
19550 13:47:59 -  -   Error:           [OMSIPresence] Exception 0EEDFADE at 7630D902 in module KERNELBASE.dll. Saved to OMSIPresence_134756.dmp

The three repeating logs have nothing to do with OMSIPresence - I think it's caused by a misconfigured ailists.cfg, but I didn't have the time to investigate. Nonetheless, they have likely contributed to the crash, as it is the only way I can consistently reproduce it.

One potential fix is to only handle exceptions from the main thread (which is where our TTimer will run anyways) by checking and comparing thread IDs. I am currently experimenting with this fix and it seems to be working, thus it will be pushed for the next update, which will happen sooner or later depending on the fix's success rate and/or amount of (additional) complaints.

This probably deserves its own separate issue, but, at some point, it might be worth looking into other methods of handling exceptions. Dropping exception handling is not an option, as it would make debugging the plugin much more difficult, as OMSI does not care about exceptions in our plugin.

If you are unsure whether this is your crash, feel free to create a new issue - you will be referenced to this one if it is.

brokenphilip commented 1 year ago

Fixed as of 1.1.