> On Nov. 21, 2012, 2:57 p.m., Andrew Stitcher wrote:
> > This is not an acceptable change to me. The problem is purely a windows one 
> > and the assert provides a valuable check for the Unix platform.
> > 
> > Basically I think the windows implementation of PollableQueue is just wrong 
> > - or rather (given that it is implemented purely as a header file) that the 
> > Poller it sits on does not do the correct thing on Windows.
> > 
> > Essentially the problem it seems to me is that the code called Poller on 
> > Windows does not actually perform the Poller functionality, it merely 
> > performs a useful service to the AsynchIO code that sits on top of it. If 
> > the name was changed then there would have been no confusion and it 
> > couldn't have been used as the basis for the Windows implementation of 
> > PollableQueue, which would have needed a different implementation on 
> > Windows.
> 
> Andrew Stitcher wrote:
>     Looking at this a bit more - I think essentially the issue is that 
> PollableCondition (a completely dreadful name which tells you nothing about 
> what the class is supposed to do) needs to ensure that it doesn't dispatch 
> multiple times to PollableQueue.

Perhaps there needs to be an equivalent to the completionQueue/working 
serializing mechanism in windows::AsynchIO here for the 
PollableConditionPrivate implementation.

I always assumed that this mechanism was there so that the concurrency 
assumptions could be matched between Linux epoll and the philosophically 
differing Windows overlapped IO and completion port concepts.

The Linux/Posix poller implementations imposes the one thread per IOHandle 
rule, whereas the Windows poller is agnostic and leaves it up to the derived 
IOHandle classes.  Presumably all Windows IOHandle classes need this 
serializing ability, or it needs to be moved into IOHandle or the poller itself.


- Cliff


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8147/#review13669
-----------------------------------------------------------


On Nov. 21, 2012, 12:54 a.m., Steve Huston wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8147/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2012, 12:54 a.m.)
> 
> 
> Review request for qpid, Andrew Stitcher, Chug Rolke, and Cliff Jansen.
> 
> 
> Description
> -------
> 
> This change prevents multiple threads from processing events at the same 
> time. They can be enqueued by multiple threads but not dispatched.
> 
> 
> This addresses bug QPID-4424.
>     https://issues.apache.org/jira/browse/QPID-4424
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/PollableQueue.h
>  1411491 
> 
> Diff: https://reviews.apache.org/r/8147/diff/
> 
> 
> Testing
> -------
> 
> Original test case from QPID-4424.
> 
> 
> Thanks,
> 
> Steve Huston
> 
>

Reply via email to