On 11/17/21 2:39 AM, Mladen Turk wrote:
>
>
> On 16/11/2021 12:00, Yann Ylavic wrote:
>> On Wed, Nov 10, 2021 at 4:19 PM Yann Ylavic <ylavic....@gmail.com> wrote:
>>>
>>> Otherwise I'll revert because I have no way to test it, but I think
>>> that apr_poll_drain_wakeup_pipe() might block on Windows for the same
>>> reason as r1894914 for platforms with poll()able pipes.
>>
>> Reverted in 1.7.x, I'll leave it in trunk for now..
>>
>
> OK, did some digging.
>
> First it's an abomination that 512+ threads call apr_pollset_wakeup
> for the same pollset.
>
> When I wrote that wakeup logic, 512 bytes in drain_wakeup_pipe was a
> quick hack to make it work without adding 'already send wakeup for this
> pollset'
>
> Here is the original comment:
> /* Although we write just one byte to the other end of the pipe
> * during wakeup, multiple treads could call the wakeup.
> * So simply drain out from the input side of the pipe all
> * the data.
> */
>
> So it seems it's time to make it properly :)
>
> Basically the 'apr_file_putc(1, pollset->wakeup_pipe[1])' in
> apr_pollset_wakeup should be called only if not already called
> for that pollset.
>
> pollset should have something like 'send_wakeup' member
> that will be reset by apr_poll_drain_wakeup_pipe call.
>
> The problem is how to make thread safe.
How about using an int flag set / read with atomic operations e.g.
apr_atomic_cas32?
Atomic stuff should not add too much performance penalties.
Regards
Rüdiger
