At Tue, 17 Dec 2013 15:45:53 +0800, Liu Yuan wrote: > > On Tue, Dec 17, 2013 at 04:26:09PM +0900, Hitoshi Mitake wrote: > > At Tue, 17 Dec 2013 15:11:30 +0800, > > Liu Yuan wrote: > > > > > > On Tue, Dec 17, 2013 at 03:58:07PM +0900, Hitoshi Mitake wrote: > > > > At Tue, 17 Dec 2013 14:50:19 +0800, > > > > Liu Yuan wrote: > > > > > > > > > > On Tue, Dec 17, 2013 at 03:42:29PM +0900, Hitoshi Mitake wrote: > > > > > > At Tue, 17 Dec 2013 14:31:56 +0800, > > > > > > Liu Yuan wrote: > > > > > > > > > > > > > > This allow us to call even handling functions in worker thread > > > > > > > > > > > > > > Signed-off-by: Liu Yuan <namei.u...@gmail.com> > > > > > > > --- > > > > > > > > > > > > I think this change is dangerous. This permits worker threads to > > > > > > unregister events even if these events are processed in the main > > > > > > thread. Making a new work queue and delegate it to > > > > > > register/unregister > > > > > > events would be safer. > > > > > > > > > > This scheme is not pratical for async request. > > > > > > > > > > This is just internal API, which are supported to be called by > > > > > programmers > > > > > and check it is correct. > > > > > > > > The checking will cost us lots of time. The problem is a race > > > > condition caused by multiple threads. > > > > > > Why we would face this kind of problem? we can make sure at any time, > > > there will > > > be a single thread manipulate it, be it worker or main thread. > > > > > > I think we are talking about different issues, you are supposed that > > > events > > > handling will be generically thread-safe for multiple threads. But this > > > wouldn't > > > happen I think. Instead, assumption 'one event will be only be > > > manipulated by > > > single entity (thus no multiple threads case)' will hold true in the long > > > run. > > > > As you say, your patch doesn't violate the above condition (one event will > > be > > only be manipulated by single entity). But we cannot express that > > register_event() and unregister_event() are thread safe in the above special > > condition. So users (we programmers) would use in an invalid way in the > > future > > and debugging will be hard. > > Before anyone tries to violet the assumption, we can stop it by telling them > not > to. And reg/unreg is very low level API and wouldn't have many users of it. So > every calling of them can be strictly checked and no burder for most of future > patches.
I think we must not be overconfident in our attentiveness. We need a mechanical method for doing this sort of checking. If you add a FIXME comment to the patch, I'll agree it. I'll seek a safer way with my internal improvements of event loop. Thanks, Hitoshi -- sheepdog mailing list sheepdog@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/sheepdog
