Hello Hans, 04.07.2017 08:55, Hans de Goede wrote: [Discussion of inconsistent wait event cancelling code in VBoxGuest.] > So some remarks about this, I was thinking that *maybe* the old > behavior is on purpose, imagine the following: > > 1) cancel, no waiters, pSession->fPendingCancelWaitEvents get set > 2) the thread for which the cancel was intended stops, without ever > calling into vgdrvIoCtl_WaitEvent again and so does not clear > the fPendingCancelWaitEvents flag > 3) (much) later a new thread in the same session starts waiting > > old behavior: new thread sees an empty event, goes back to > waiting > > new behavior: new thread sees a VERR_INTERRUPTED -> and stops > ? (note I've not looked at the userspace code).
I think you are reading too much intelligent design into the interface. I was the guilty person who created it, long ago, and I don't think I thought it through sufficiently well at the time. That said, I don't think I wrote the current implementation code. I looked at the user-space code I mentioned again, and actually realised that it does handle the old (and the new) code. Testing seems to confirm this. > ##################################################################### > > One other thing worth noticing is that the current wait/cancel > code causes events to get lost at cancel, imagine the following > sequence: > > 1) Some events get dispatched, moved from pDevExt->f32PendingEvents to > pWait->fResEvents > 2) vgdrvIoCtl_CancelAllWaitEvents runs replacing pWait->fResEvents with > UINT32_MAX > 3) vgdrvIoCtl_WaitEvent consumes pWait->fResEvents, sees UINT32_MAX, > treats it as > a cancel. > > Note this is something which I plan to fix in my rewrite of this code > to directly use linux wait_queues (which will make the code much much > smaller). > > Actually mimicking this bug will be hard to do :) [...] I really don't think you need to mimic this. Since CANCEL_ALL_WAITEVENTS is currently only used for cleanly stopping processes, it is not likely to get noticed either way. That said, it might be worth us fixing it in case this ever changes. Regards Michael -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher _______________________________________________ vbox-dev mailing list [email protected] https://www.virtualbox.org/mailman/listinfo/vbox-dev
