----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42647/#review116070 -----------------------------------------------------------
proton-c/bindings/python/cproton.i (line 409) <https://reviews.apache.org/r/42647/#comment177087> I'm confused by the comment... Can we remove this and just have interrupt() on the Reactor? proton-c/bindings/python/cproton.i (line 447) <https://reviews.apache.org/r/42647/#comment177088> 'async_notify' is a horrible name... is this in addition to or instead of on_interrupt? proton-c/bindings/python/proton/reactor.py (line 232) <https://reviews.apache.org/r/42647/#comment177089> In the modified code, what is _closed ever used for? proton-c/include/proton/reactor.h (line 90) <https://reviews.apache.org/r/42647/#comment177090> Do we need to be able to pass in a handler? Why not just use whatever handler the reactor si configured with? proton-c/src/events/event.c (line 140) <https://reviews.apache.org/r/42647/#comment177091> It would be worth a comment here explaining why PN_INTERRUPT is a special case. proton-c/src/events/event.c (line 385) <https://reviews.apache.org/r/42647/#comment177092> I'd be inclined to make this a reactor scoped event i.e. PN_REACTOR_INTERRUPT proton-c/src/posix/selector.c (line 68) <https://reviews.apache.org/r/42647/#comment177080> Would be safer to set the fds to -1 after closing. proton-c/src/posix/selector.c (line 82) <https://reviews.apache.org/r/42647/#comment177098> Does this mean we are creating a pipe not just for each reactor but for every connection within it? That doesn't seem right if so... Since the interrupt is now essentially a special method on the reactor, EventInjector as a separate, user-visiable class doesn't make as much sense in my view. I'd be inclined to try and merge it into Container. - Gordon Sim On Jan. 22, 2016, 8:11 a.m., Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42647/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2016, 8:11 a.m.) > > > Review request for qpid, Alan Conway, Andrew Stitcher, Gordon Sim, and Justin > Ross. > > > Bugs: PROTON-1071 > https://issues.apache.org/jira/browse/PROTON-1071 > > > Repository: qpid-proton-git > > > Description > ------- > > Wrote new thread safe primitives for reactor and selector for Windows and > adapted the existing posix self-pipe mechanism to the changed API. > > Reworked the Python EventInjector to use these primitives via special swig > functions. > > Updated the io.h documentation to reflect the more conservative thread > guarantee. > > > Diffs > ----- > > examples/python/db_recv.py 8c79049 > examples/python/db_send.py c07dcc0 > proton-c/bindings/python/cproton.i 734069f > proton-c/bindings/python/proton/reactor.py 195ff28 > proton-c/include/proton/event.h 16d2bda > proton-c/include/proton/io.h 19dfe53 > proton-c/include/proton/reactor.h e91b169 > proton-c/include/proton/selector.h c942393 > proton-c/src/events/event.c 5ad718e > proton-c/src/posix/selector.c 7f72c84 > proton-c/src/reactor/reactor.c 7ea279b > proton-c/src/windows/iocp.h 0e052e5 > proton-c/src/windows/iocp.c 404dd36 > proton-c/src/windows/selector.c f139aec > tests/python/proton_tests/reactor.py 6ee107d > > Diff: https://reviews.apache.org/r/42647/diff/ > > > Testing > ------- > > rhel7, windows8 > > > Thanks, > > Cliff Jansen > >
