search

microsoft / MIDI

Windows MIDI Services
MIT License
285 stars 23 forks source link

[BUG] Sending messages without monitoring causes Green Screen with the USB MIDI 2.0 driver #307

Open m-komo opened 2 months ago

m-komo commented 2 months ago

Describe the bug Sending a lots of messages without monitoring causes Green Screen. This issue only occurs if the device is running with the USB MIDI 2.0 driver (UsbMidi2.sys)

To Reproduce

  1. Update ProtoZOA with the attached firmware.
  2. Attach ProtoZOA to PC and update the driver to the USB MIDI 2.0 driver (UsbMidi2.sys).
  3. Open console and make sure there is no monitoring command (midi endpoint monitor) running.
  4. Run "midi endpoint send-message 0x40123456 0x00000000 -p 0 -c 125000 -i".
  5. Choose "ZOA-216" as an endpoint.
  6. Repeat step 4 to 5 several times.

Green Screen occurs. I faced two types of Green Screen, "ATTEMPTED_SWITCH_FROM_DPC" and "KERNEL_SECURITY_CHECK_FAILURE".

Expected behavior No Green Screen.

Installer Name or Version

Desktop (please complete the following information):

Device information, if this is with an external MIDI device:

MusicMaker commented 2 months ago

https://www.windowslatest.com/2024/04/06/windows-11-24h2-build-26100-causes-undocumented-issues-blocks-some-apps/ FYI 26001 is reported to be buggy with several GSODs.. I can't even do Windows update to it anymore ATM. (stuck at an older version which by itself is also a known issue and suggests a clean install)

MusicMaker commented 2 months ago

Finally installed 26100.1 (after using a network cable and several retires)

Loaded that FW in an off the shelf RP2040 board , assigned the midi preview 5 driver and ran that command many times. it sens the data, Cannot reproduce the GSOD.

MusicMaker commented 2 months ago

Got two crashes now of Windows 11. (auto reboots)... but not when running the command but with that RP2040 connected after running the commands. need further check if related. Now fully reproduceable. indeed an GSOD

Psychlist1972 commented 2 months ago

Thank you for the repro and the crash dump, @MusicMaker.

https://www.windowslatest.com/2024/04/06/windows-11-24h2-build-26100-causes-undocumented-issues-blocks-some-apps/ FYI 26001 is reported to be buggy with several GSODs.. I can't even do Windows update to it anymore ATM. (stuck at an older version which by itself is also a known issue and suggests a clean install)

If you want to get off the Canary train, I was told Dev channel has the ACX / USB updates in it now, so USB MIDI 2 will work on it.

MusicMaker commented 2 months ago

Indeed. noticed the Dev channel works too. Regress it with next canary build....

AmeNote-Michael commented 2 months ago

Needs further investigation and analysis.

@garydan42 thinking about it, the act of midi endpoint send-message will put driver into "run" state which will process the USB IN buffer and into double buffer. As there is no monitor or application, nothing will be feeding from double buffer. In code, the reaction is to drop data but then request to handle existing data.

We should actually respond with discarding oldest data and then putting new data in buffer. What is the design pattern for this?

However, does not explain GSOD - just noting what we should be doing.

AmeNote-Michael commented 1 week ago

@garydan42 This issue is due to the "Run" state being set when a send-message occurs but nothing is reading from driver. If the device has data provided and the driver is in run state, driver will populate the double buffer stream for UMP IN with UMP data. If there is nothing reading from the UMP Data from that stream, it will eventually fill.

I suspect we should just drop old data - but what is design pattern here? How do I ensure I am not conflicting on memory with other software that may be accessing stream? Is it same as when dealing with buffer for write?

garydan42 commented 1 day ago

The pin instance shouldn't be in the run state unless someone is retrieving buffers from the cross-process queue. The code that instantiates the cross-process buffer also creates the worker threads that does the reads and callbacks. The service should be retrieving these messages, and if there are no clients listening, dropping them.

That's not to say that the cross-process queue can't become full because messages are arriving faster than they can be processed or the callback blocks or hangs. The expectation is that the newest messages are dropped in that case, not that the oldest are dropped. Architecturally there's no way to drop the oldest messages because it's a producer/consumer fifo with the reader owning removing messages from the front of the fifo and the writer adding messages to the end. To drop the oldest messages the current read position would have to be advanced by the writer thread, but the reader may be actively reading at the current read position and modifying it would lead to data corruption.

Sent from Outlookhttp://aka.ms/weboutlook

From: Michael Loh @.> Sent: Monday, June 24, 2024 5:31 PM To: microsoft/MIDI @.> Cc: Mention @.>; Assign @.> Subject: Re: [microsoft/MIDI] [BUG] Sending messages without monitoring causes Green Screen with the USB MIDI 2.0 driver (Issue #307)

@garydan42https://github.com/garydan42 This issue is due to the "Run" state being set when a send-message occurs but nothing is reading from driver. If the device has data provided and the driver is in run state, driver will populate the double buffer stream for UMP IN with UMP data. If there is nothing reading from the UMP Data from that stream, it will eventually fill.

I suspect we should just drop old data - but what is design pattern here? How do I ensure I am not conflicting on memory with other software that may be accessing stream? Is it same as when dealing with buffer for write?

— Reply to this email directly, view it on GitHubhttps://github.com/microsoft/MIDI/issues/307#issuecomment-2187443202 or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOXSFVA77GNRVMTM37KQLBLZJCF3VBFKMF2HI4TJMJ2XIZLTTGBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVI2DCNRUGYZDIMBVHCSG4YLNMWUWQYLTL5WGCYTFNSBKK5TBNR2WLKRUGE4DAMBQGQZDQM5ENZQW2ZNJNBQXGX3MMFRGK3ECUV3GC3DVMWVDKNRTGU2DGNJRGA3KI3TBNVS2S2DBONPWYYLCMVWIFJLWMFWHKZNKGU3DIOJSGYYTMMBYURXGC3LFVFUGC427NRQWEZLMQKSXMYLMOVS2UNRYGMZDAMJUHA2DRJDOMFWWLKLIMFZV63DBMJSWZAVFOZQWY5LFVI3DSOBVG44DQMRQGOSG4YLNMWUWQYLTL5WGCYTFNSWHG5LCNJSWG5C7OR4XAZNMJFZXG5LFINXW23LFNZ2KM5DPOBUWG44YQKSHI6LQMWVHEZLQN5ZWS5DPOJ42K5TBNR2WLKJUHE2TSNJZGQ3TDAVEOR4XAZNFNFZXG5LFUV3GC3DVMWVDEMRTG4ZDGNBTHE2YFJDUPFYGLJLMMFRGK3FFOZQWY5LFVI2DCNRUGYZDIMBVHCBKI5DZOBS2K3DBMJSWZJLWMFWHKZNKGQYTQMBQGA2DEOBTQKSHI6LQMWSWYYLCMVWKK5TBNR2WLKRVGYZTKNBTGUYTANUCUR2HS4DFUVWGCYTFNSSXMYLMOVS2UNJWGQ4TENRRGYYDRAVEOR4XAZNFNRQWEZLMUV3GC3DVMWVDMOBTGIYDCNBYGQ4IFJDUPFYGLJLMMFRGK3FFOZQWY5LFVI3DSOBVG44DQMRQGOTXI4TJM5TWK4VGMNZGKYLUMU. You are receiving this email because you were mentioned.

Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.