nomi-san / parsec-vdd

✨ Perfect 4K@240Hz Virtual Display
MIT License
1.78k stars 89 forks source link

Fix infinite blocking in VddIoControl in case of a driver crash #52

Closed timminator closed 2 weeks ago

timminator commented 2 weeks ago

Hi! I have noticed a problem in the event of a driver crash of the parsec vdd driver. The "GetOverlappedResult" function in VddIoControl blocks indefinitely in this case because the operation does not complete. This causes VddIoControl to never return and as a result the display updater thread cannot be joined. I found a solution for this problem by using the "GetOverlappedResultEx" function instead with a relatively high timeout of 5 seconds, that can only be triggered in the event of a driver crash. This ensures that if the operation does not complete in the specified timeout, the function will not block indefinitely and the display updater thread can now be joined in this event.

Edit: Resolved merge conflicts

2nd edit: I scrolled through your code and noticed that you are using GetOverlappedResultEx with a timeout in ParsecVDD.cs already, but it is not used in the parsec-vdd.h file.

nomi-san commented 2 weeks ago

Thank you. I have forgotten this header. I see the change of CreateEventA to manual-reset (TRUE), but it does not matter because there's no any wait functions around GetOverlappedResultEx. Parsec also uses auto-reset event.

-    Overlapped.hEvent = CreateEventA(NULL, FALSE, FALSE, NULL);
+    Overlapped.hEvent = CreateEventA(NULL, TRUE, FALSE, NULL);

Specify a manual-reset event object in the OVERLAPPED structure. If an auto-reset event object is used, the event handle must not be specified in any other wait operation in the interval between starting the overlapped operation and the call to GetOverlappedResultEx.

timminator commented 1 week ago

Thank you for your note regarding the manual reset parameter. When i tried to resolve this issue, i attempted quite a few things including a WaitForSingleObject function and i only got it working after changing the manual reset parameter to true. I somehow thought it was needed for the GetOverlappedResultEx function to work correctly, but after reading your comment, specifically "because there`s no any wait functions", it clicked in my head that this changed was needed due to the WaitForSingleObject function in combination with GetOverlappedResultEx and not for adding a timeout. I also testet it again, and it is indeed working without this change. Thank you! I made a commit to my branch and rolled back this unnecessary change. Should i open a new commit or is it not important?

nomi-san commented 1 week ago

No problem, but no necessary to rollback now, I think it could be used in the future. 😅