On Mon, 09/25 13:50, Peter Xu wrote: > On Mon, Sep 25, 2017 at 01:30:02PM +0800, Fam Zheng wrote: > > On Mon, 09/25 13:23, Peter Xu wrote: > > > On Fri, Sep 22, 2017 at 09:09:22PM +0800, Fam Zheng wrote: > > > > On Fri, 09/22 16:56, Peter Xu wrote: > > > > > When gcontext is used with iothread, the context will be destroyed > > > > > during iothread_stop(). That's not good since sometimes we would like > > > > > to keep the resources until iothread is destroyed, but we may want to > > > > > stop the thread before that point. > > > > > > > > Would be nice if you can also mention the glib bug that "required" this > > > > in the > > > > commit message. > > > > > > I can add it, but I am not sure it's very closely related (and I'm > > > afraid that may confuse more people). Say, even without that bug, I > > > would still think it not a good idea to free the context in the loop, > > > especially considering that we have the finalize function there. Thanks, > > > > It's interesting to know if or not your future change will break without > > this > > patch, this is especially useful for backport. > > I haven't tried to run with iothread and without this patch, but I > think it should fail, so this patch should be needed. > > The point is that we should not destroy the context before explicitly > calling remove_fd_in_watch() if the context is running chardevs. > Without this patch, this rule does not satisfy. And IIUC this rule > comes from the glib bug. > > Anyway, I'll mention it in commit message to clarify.
OK, thanks for the explanations! My r-b still stands with the amended commit log. Fam
