On Tue, Nov 28, 2017 at 05:51:21PM +0100, Paolo Bonzini wrote: > On 28/11/2017 17:42, Jeff Cody wrote: > > On Tue, Nov 28, 2017 at 05:28:50PM +0100, Kevin Wolf wrote: > >> Am 28.11.2017 um 17:14 hat Paolo Bonzini geschrieben: > >>> On 28/11/2017 16:43, Kevin Wolf wrote: > >>>> + /* Make sure that a coroutine that can alternatively reentered from > >>>> two > >>>> + * different sources isn't reentered more than once when the first > >>>> caller > >>>> + * uses aio_co_schedule() and the other one enters to coroutine > >>>> directly. > >>>> + * This is achieved by cancelling the pending aio_co_schedule(). > >>>> + * > >>>> + * The other way round, if aio_co_schedule() would be called after > >>>> this > >>>> + * point, this would be a problem, too, but in practice it doesn't > >>>> happen > >>>> + * because we're holding the AioContext lock here and > >>>> aio_co_schedule() > >>>> + * callers must do the same. > >>> > >>> No, this is not true. aio_co_schedule is thread-safe. > >> > >> Hm... With the reproducer we were specfically looking at > >> qmp_block_job_cancel(), which does take the AioContext locks. But it > >> might not be as universal as I thought. > >> > >> To be honest, I just wasn't sure what to do with this case anyway. It > >> means that the coroutine is already running when someone else schedules > >> it. We don't really know whether we have to enter it a second time or > >> not. > >> > >> So if it can indeed happen in practice, we need to think a bit more > >> about this. > > > > It would be nice if, on coroutine termination, we could unschedule all > > pending executions for that coroutine. I think use-after-free is the main > > concern for someone else calling aio_co_schedule() while the coroutine is > > currently running. > > Yes, terminating a scheduled coroutine is a bug; same for scheduling a > terminated coroutine, both orders are wrong. However, "unscheduling" is > not the solution; you would just be papering over the issue. >
Maybe we should at least add an abort on coroutine termination if there are still outstanding schedules, as that is preferable to operating in the weeds. > aio_co_schedule() on a running coroutine can only happen when the > coroutine is going to yield soon. > That is a bit vague. What is "soon", and how does an external caller know if a coroutine is going to yield in this timeframe? Jeff
