[
https://issues.apache.org/jira/browse/PROTON-1071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15045851#comment-15045851
]
Cliff Jansen commented on PROTON-1071:
--------------------------------------
This JIRA cuts across a number of Proton IO and threading issues.
EventInjector as coded violates Proton's io model: the pipe needs to come from
pn_pipe() and the reading and writing via pn_read() and pn_write(), all on the
reactor's pn_io_t object. It works by fortuitous accident on Posix.
Coding EventInjector to work this way for Posix is presumably simple. For
Windows, it further requires tackling deferred thread safety, at least for
pipes and the associated pn_io_t->selector (but that may pull in everything).
As noted in PROTON-640, there is no thread safety in Proton io on Windows other
than a weak guarantee that two separate threads may independently work on
separate pn_io_t objects.
PROTON-688 attempted to document the stronger but still limited thread-safety
available in the Posix implementation of Proton io. This is the model of
concurrency used by Qpid Dispatch Router and perhaps the model that the Windows
implementation should strive to. There is an opposing point of view that
pushing thread safety into Proton is counterproductive: applications know what
threading/io model works best for their workload. Hence the rising interest in
connection_engine.
EventInjector seems like a pretty important use case for coordinating threads.
Alternatively, a more limited (or dedicated) api extension, perhaps
pn_reactor_inject() which does platform-specific thread-safe coordination of
something to be collected and the pn_io_t->selector might be more clear in its
intention (as opposed to knowing that pn_write/read/select work a certain way
together).
The Windows io.c and related code could be made to work at the same level of
thread safety as Posix, but with obvious locking overhead. If the assumption
is that an application needing high performance IO will just use the
connection_engine and manage the IO itself, perhaps there is no need to care
about the locking penalty.
See also PROTON-889 regarding general thread safety outages in Proton on all
platforms for pn_io_error() and pn_io_wouldblock().
> EventInjector hangs on Windows
> ------------------------------
>
> Key: PROTON-1071
> URL: https://issues.apache.org/jira/browse/PROTON-1071
> Project: Qpid Proton
> Issue Type: Bug
> Components: proton-c, python-binding
> Affects Versions: 0.11
> Environment: Windows
> Reporter: Ken Giusti
> Assignee: Cliff Jansen
> Fix For: 0.12.0
>
>
> I added a new reactor test that exercises the python-proton ApplicationEvent
> and EventInjector classes:
> proton_tests.reactor.ApplicationEventTest.test_application_events
> See tests/python/proton_tests/reactor.py
> This test passes on linux, but hangs when run on Windows.
> Poking around a bit, I suspect the problem may be in the Windows selector
> code. Description:
> The EventInjector/ApplicationEvent classes provide a way to trigger events
> from threads external to the reactor main loop. See
> proton-c/bindings/python/proton/reactor.py. A pipe is used to wake up the
> reactor when a new event is sent to the reactor (see reactor.py in the python
> bindings). The EventInjector's trigger method puts the event on a queue and
> writes to a pipe to wake up the reactor. The on_selectable_readable callback
> in the EventInjector is called on the reactor thread to get the event off the
> queue and clear the pipe.
> On windows it appears as if the EventInjector selectable is made "readable"
> even though nothing has been written to the pipe. This causes the os.read()
> call in the on_selectable_readable() callback to hang.
> Best I can tell the windows selector code doesn't work properly with a pipe.
> The pn_selector_next() function is returning a read event on the pipe's read
> descriptor even though the pipe is empty. But I'm not familiar with the
> window's selector implementation, so this is a best guess.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
