You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modules-dev@httpd.apache.org by Alex Bligh <al...@alex.org.uk> on 2012/09/09 14:31:31 UTC

Bucket brigade & filter thread safety

I am trying to work out how to develop a thread-safe module with two
threads, one thread reading and one thread writing. I'm using mpm-prefork
on apache 2.2.14-5ubuntu8.9 in case that matters. The module is a websocket
proxy.

My main thread (#1) is doing (in essence - error handling removed)

apr_pool_create(&opool, r->pool);
apr_bucket_alloc_t *oallocator = apr_bucket_alloc_create(opool);
apr_bucket_brigade *obb = apr_brigade_create(opool, oallocator);
apr_thread_create (... otherthread ...);
while (1) {
    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
    ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
                   APR_BLOCK_READ, bufsiz));
    apr_brigade_flatten(bb, buffer, &bufsiz);
    apr_brigade_destroy(bb);
    do_something_with(buffer);
}

The other thread (#2) is doing (in essence)

while (1) {
   apr_pollset_poll(recvpollset, timeout, ... );
   apr_socket_recv(socket, buf, ...)
   do_something_else_with(buf);
   ap_fwrite(r->connection->output_filters, obb, ...);
   ap_fflush(r->connection->output_filters, obb, ...);
}

I am suffering from very occasional corruption of the bucket brigade which
normally shows up as a corrupted pool pointer or a bogus bucket entry in
thread #1 (for instance a SEGV in apr_brigade_length). Interestingly this
is the relatively quiet input brigade which is only ever touched by the
main apache thread. It's almost as if an allocator is not thread safe.
However, I'm using a separate bucket allocator (see code above) and (at
least in my code) a separate pool.

Could the output filter chain somehow be using the allocator attached to
the request (i.e. thread #1)? If so, how can I stop this? I can't run the
ap_fwrite/ap_flush under a mutex and have the same mutex held during
ap_get_brigade, as the latter blocks (and I can't see how to use the
non-blocking version without spinning).

[apologies for the partial dupe on the apache-users mailing list - I think
I've got nearer the problem since then and this list seems more
appropriate]

-- 
Alex Bligh

Re: Bucket brigade & filter thread safety

Posted by Ben Noordhuis <in...@bnoordhuis.nl>.
On Mon, Sep 10, 2012 at 3:53 PM, Alex Bligh <al...@alex.org.uk> wrote:
> Ben,
>
>
>> No, but the documentation omits some crucial details.
>> apr_pool_create() is thread-safe only if:
>>
>> 1. libapr is compiled with APR_HAS_THREADS
>> 2. APR_POOL_DEBUG is turned off
>> 3. the parent pool has a thread-safe allocator (which is true for the
>> global allocator that is used when parent=NULL, provided conditions #1
>> and #2 are satisfied)
>>
>> The pools you get from httpd core satisfy #3 but a module may replace
>> e.g. r->pool with another pool that doesn't. Ergo, don't rely on a
>> pool being thread safe unless you explicitly make it so.
>
>
> Ah, OK. I had thought that was only an issue as when the pool create
> was running (at which point I am single threaded). But I can fix
> that - thanks.
>
>
>>>> That won't solve all your problems though. Bucket brigades are not
>>>> thread safe, you will need something to synchronize on.
>>>
>>>
>>>
>>> So what I was trying to do was to use
>>> a) the input bucket brigade in thread #1 (main thread)
>>> b) the output bucket brigade in thread #2
>>> in an attempt to avoid synchronization
>>>
>>> But what I don't understand is whether thread #2, in writing
>>> to the output filters (which presumably have a reference
>>> to r->pool) will need synchronisation.
>>
>>
>> Yes. It's not just because r->pool may or may not be synchronized, the
>> internal structure of the bucket brigade is not protected by any locks
>> either.
>
>
> Oh I understand that, but I thought in the example above only
> thread #1 would be accessing the input bucket brigade and
> only thread #2 would be accessing the output bucket brigade,
> so there would be no need for synchronisation as they were
> thread local.
>
>
>>> And if I have to synchronize, how do I do that in practice?
>>> Thread #2 does and ap_fwrite/ap_flush so I can hold a mutex
>>> there. But what do I do in thread #1, which calls ap_brigade_get
>>> and blocks? I can't hold a mutex during that. I can make it
>>> a non-blocking ap_brigade_get (if I understood how to do it)
>>
>>
>> Non-blocking reads are pretty straightforward:
>>
>>   apr_thread_mutex_lock(&mutex);
>>   rv = ap_get_brigade(f->next, bb, AP_MODE_READBYTES, APR_NONBLOCK_READ,
>> len);   if (APR_STATUS_IS_EAGAIN(rv)) apr_thread_cond_wait(&cond, &mutex);
>>   rv = ap_get_brigade(f->next, bb, AP_MODE_READBYTES, APR_NONBLOCK_READ,
>> len);   apr_thread_mutex_unlock(&mutex);
>>
>> The other thread wakes up this thread with apr_thread_cond_signal(&cond).
>
>
> I think I may not have explained what I am doing clearly. One thread
> is doing input (from the apache client), and the other output (to
> the apache client). The ap_brigade_get is in the input thread (the
> main thread) and is blocking on the client sending more data. So
> the thing that would need to wake the thread up is more data becoming
> ready from the client - nothing to do with the other
> thread. I don't know how to detect that.
>
>
>>> but what I really need is the equivalent of a select() which
>>> I can do with the mutex not held (or some way to drop the mutex
>>> during the raw reads). Any ideas?
>>
>>
>> You could set up a pollset in the main thread and funnel incoming data
>> into your bucket brigade. Not terribly efficient (lots of context
>> switches) but the real world impact may very well be negligible and
>> you can support multi-process setups with zero changes to your code.
>
>
> Hmm, well if I could use a pollset in my main thread I wouldn't
> need bucket brigades at all. But as this is data coming from the
> client, surely it's going to be in a bucket brigade already as
> it will have passed through all the input filters etc., having
> been read by apache itself?
>
> Diagramatically:
>
>
>             | APACHE  |   ........... MODULE ........... |
>
>  Client <==> Apache ----> ap_get_brigade ----> do_something  [thread1]
>                   ^
>                   |------ ap_fwrite <--- do_something_else   [thread2]
>
> Each thread has a different bucket brigade with a different allocator.
>
> ap_get_brigade either needs to block, or if it's non-blocking it
> needs to wait on a condwait or something for /apache/ to produce
> more data from the client, not on the other thread.

Right, I think I see what you mean.

Apache may not be a perfect fit. I once had to solve a similar issue
and I eventually settled on sending the socket to another process.
Managing mostly dormant connections turned out to be too much of a
headache to do from within httpd.

Re: Bucket brigade & filter thread safety

Posted by Alex Bligh <al...@alex.org.uk>.
Ben,

> No, but the documentation omits some crucial details.
> apr_pool_create() is thread-safe only if:
>
> 1. libapr is compiled with APR_HAS_THREADS
> 2. APR_POOL_DEBUG is turned off
> 3. the parent pool has a thread-safe allocator (which is true for the
> global allocator that is used when parent=NULL, provided conditions #1
> and #2 are satisfied)
>
> The pools you get from httpd core satisfy #3 but a module may replace
> e.g. r->pool with another pool that doesn't. Ergo, don't rely on a
> pool being thread safe unless you explicitly make it so.

Ah, OK. I had thought that was only an issue as when the pool create
was running (at which point I am single threaded). But I can fix
that - thanks.

>>> That won't solve all your problems though. Bucket brigades are not
>>> thread safe, you will need something to synchronize on.
>>
>>
>> So what I was trying to do was to use
>> a) the input bucket brigade in thread #1 (main thread)
>> b) the output bucket brigade in thread #2
>> in an attempt to avoid synchronization
>>
>> But what I don't understand is whether thread #2, in writing
>> to the output filters (which presumably have a reference
>> to r->pool) will need synchronisation.
>
> Yes. It's not just because r->pool may or may not be synchronized, the
> internal structure of the bucket brigade is not protected by any locks
> either.

Oh I understand that, but I thought in the example above only
thread #1 would be accessing the input bucket brigade and
only thread #2 would be accessing the output bucket brigade,
so there would be no need for synchronisation as they were
thread local.

>> And if I have to synchronize, how do I do that in practice?
>> Thread #2 does and ap_fwrite/ap_flush so I can hold a mutex
>> there. But what do I do in thread #1, which calls ap_brigade_get
>> and blocks? I can't hold a mutex during that. I can make it
>> a non-blocking ap_brigade_get (if I understood how to do it)
>
> Non-blocking reads are pretty straightforward:
>
>   apr_thread_mutex_lock(&mutex);
>   rv = ap_get_brigade(f->next, bb, AP_MODE_READBYTES, APR_NONBLOCK_READ,
> len);   if (APR_STATUS_IS_EAGAIN(rv)) apr_thread_cond_wait(&cond, &mutex);
>   rv = ap_get_brigade(f->next, bb, AP_MODE_READBYTES, APR_NONBLOCK_READ,
> len);   apr_thread_mutex_unlock(&mutex);
>
> The other thread wakes up this thread with apr_thread_cond_signal(&cond).

I think I may not have explained what I am doing clearly. One thread
is doing input (from the apache client), and the other output (to
the apache client). The ap_brigade_get is in the input thread (the
main thread) and is blocking on the client sending more data. So
the thing that would need to wake the thread up is more data becoming
ready from the client - nothing to do with the other
thread. I don't know how to detect that.

>> but what I really need is the equivalent of a select() which
>> I can do with the mutex not held (or some way to drop the mutex
>> during the raw reads). Any ideas?
>
> You could set up a pollset in the main thread and funnel incoming data
> into your bucket brigade. Not terribly efficient (lots of context
> switches) but the real world impact may very well be negligible and
> you can support multi-process setups with zero changes to your code.

Hmm, well if I could use a pollset in my main thread I wouldn't
need bucket brigades at all. But as this is data coming from the
client, surely it's going to be in a bucket brigade already as
it will have passed through all the input filters etc., having
been read by apache itself?

Diagramatically:


             | APACHE  |   ........... MODULE ........... |

  Client <==> Apache ----> ap_get_brigade ----> do_something  [thread1]
                   ^
                   |------ ap_fwrite <--- do_something_else   [thread2]

Each thread has a different bucket brigade with a different allocator.

ap_get_brigade either needs to block, or if it's non-blocking it
needs to wait on a condwait or something for /apache/ to produce
more data from the client, not on the other thread.

-- 
Alex Bligh

Re: Bucket brigade & filter thread safety

Posted by Ben Noordhuis <in...@bnoordhuis.nl>.
On Mon, Sep 10, 2012 at 10:47 AM, Alex Bligh <al...@alex.org.uk> wrote:
> Ben,
>
> Thanks for your reply.
>
>
>>> I am suffering from very occasional corruption of the bucket brigade
>>> which normally shows up as a corrupted pool pointer or a bogus bucket
>>> entry in thread #1 (for instance a SEGV in apr_brigade_length).
>>> Interestingly this is the relatively quiet input brigade which is only
>>> ever touched by the main apache thread. It's almost as if an allocator
>>> is not thread safe.
>>
>>
>> That's because it isn't unless you explicitly make it so (which no MPM
>> does).
>
>
> What I'm trying to do is to use a separate allocator per thread.
>
>
>>> However, I'm using a separate bucket allocator (see code above) and (at
>>> least in my code) a separate pool.
>>
>>
>> They're not really separate, the sub pool is created off r->pool. You
>> should probably use apr_pool_create_ex() here with parent=NULL and an
>> allocator that you created with apr_allocator_create() +
>> apr_allocator_mutex_set().
>
>
> OK I based this on the docs for apr_pool_create which says at
> http://apr.apache.org/docs/apr/1.4/group__apr__pools.html#ga918adf3026c894efeae254a0446aed3b
>
> : This function is thread-safe, in the sense that multiple threads can
> : safely create subpools of the same parent pool concurrently. Similarly, a
> : subpool can be created by one thread at the same time that another thread
> : accesses the parent pool.
>
> Have I understood that wrong?

No, but the documentation omits some crucial details.
apr_pool_create() is thread-safe only if:

1. libapr is compiled with APR_HAS_THREADS
2. APR_POOL_DEBUG is turned off
3. the parent pool has a thread-safe allocator (which is true for the
global allocator that is used when parent=NULL, provided conditions #1
and #2 are satisfied)

The pools you get from httpd core satisfy #3 but a module may replace
e.g. r->pool with another pool that doesn't. Ergo, don't rely on a
pool being thread safe unless you explicitly make it so.

>> That won't solve all your problems though. Bucket brigades are not
>> thread safe, you will need something to synchronize on.
>
>
> So what I was trying to do was to use
> a) the input bucket brigade in thread #1 (main thread)
> b) the output bucket brigade in thread #2
> in an attempt to avoid synchronization
>
> But what I don't understand is whether thread #2, in writing
> to the output filters (which presumably have a reference
> to r->pool) will need synchronisation.

Yes. It's not just because r->pool may or may not be synchronized, the
internal structure of the bucket brigade is not protected by any locks
either.

> And if I have to synchronize, how do I do that in practice?
> Thread #2 does and ap_fwrite/ap_flush so I can hold a mutex
> there. But what do I do in thread #1, which calls ap_brigade_get
> and blocks? I can't hold a mutex during that. I can make it
> a non-blocking ap_brigade_get (if I understood how to do it)

Non-blocking reads are pretty straightforward:

  apr_thread_mutex_lock(&mutex);
  rv = ap_get_brigade(f->next, bb, AP_MODE_READBYTES, APR_NONBLOCK_READ, len);
  if (APR_STATUS_IS_EAGAIN(rv)) apr_thread_cond_wait(&cond, &mutex);
  rv = ap_get_brigade(f->next, bb, AP_MODE_READBYTES, APR_NONBLOCK_READ, len);
  apr_thread_mutex_unlock(&mutex);

The other thread wakes up this thread with apr_thread_cond_signal(&cond).

> but what I really need is the equivalent of a select() which
> I can do with the mutex not held (or some way to drop the mutex
> during the raw reads). Any ideas?

You could set up a pollset in the main thread and funnel incoming data
into your bucket brigade. Not terribly efficient (lots of context
switches) but the real world impact may very well be negligible and
you can support multi-process setups with zero changes to your code.

Re: Bucket brigade & filter thread safety

Posted by Alex Bligh <al...@alex.org.uk>.
Ben,

Thanks for your reply.

>> I am suffering from very occasional corruption of the bucket brigade
>> which normally shows up as a corrupted pool pointer or a bogus bucket
>> entry in thread #1 (for instance a SEGV in apr_brigade_length).
>> Interestingly this is the relatively quiet input brigade which is only
>> ever touched by the main apache thread. It's almost as if an allocator
>> is not thread safe.
>
> That's because it isn't unless you explicitly make it so (which no MPM
> does).

What I'm trying to do is to use a separate allocator per thread.

>> However, I'm using a separate bucket allocator (see code above) and (at
>> least in my code) a separate pool.
>
> They're not really separate, the sub pool is created off r->pool. You
> should probably use apr_pool_create_ex() here with parent=NULL and an
> allocator that you created with apr_allocator_create() +
> apr_allocator_mutex_set().

OK I based this on the docs for apr_pool_create which says at
http://apr.apache.org/docs/apr/1.4/group__apr__pools.html#ga918adf3026c894efeae254a0446aed3b

: This function is thread-safe, in the sense that multiple threads can
: safely create subpools of the same parent pool concurrently. Similarly, a
: subpool can be created by one thread at the same time that another thread
: accesses the parent pool.

Have I understood that wrong?

> That won't solve all your problems though. Bucket brigades are not
> thread safe, you will need something to synchronize on.

So what I was trying to do was to use
a) the input bucket brigade in thread #1 (main thread)
b) the output bucket brigade in thread #2
in an attempt to avoid synchronization

But what I don't understand is whether thread #2, in writing
to the output filters (which presumably have a reference
to r->pool) will need synchronisation.

And if I have to synchronize, how do I do that in practice?
Thread #2 does and ap_fwrite/ap_flush so I can hold a mutex
there. But what do I do in thread #1, which calls ap_brigade_get
and blocks? I can't hold a mutex during that. I can make it
a non-blocking ap_brigade_get (if I understood how to do it)
but what I really need is the equivalent of a select() which
I can do with the mutex not held (or some way to drop the mutex
during the raw reads). Any ideas?

-- 
Alex Bligh

Re: Bucket brigade & filter thread safety

Posted by Ben Noordhuis <in...@bnoordhuis.nl>.
On Sun, Sep 9, 2012 at 2:31 PM, Alex Bligh <al...@alex.org.uk> wrote:
> I am trying to work out how to develop a thread-safe module with two
> threads, one thread reading and one thread writing. I'm using mpm-prefork
> on apache 2.2.14-5ubuntu8.9 in case that matters. The module is a websocket
> proxy.
>
> My main thread (#1) is doing (in essence - error handling removed)
>
> apr_pool_create(&opool, r->pool);
> apr_bucket_alloc_t *oallocator = apr_bucket_alloc_create(opool);
> apr_bucket_brigade *obb = apr_brigade_create(opool, oallocator);
> apr_thread_create (... otherthread ...);
> while (1) {
>    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>    ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
>                   APR_BLOCK_READ, bufsiz));
>    apr_brigade_flatten(bb, buffer, &bufsiz);
>    apr_brigade_destroy(bb);
>    do_something_with(buffer);
> }
>
> The other thread (#2) is doing (in essence)
>
> while (1) {
>   apr_pollset_poll(recvpollset, timeout, ... );
>   apr_socket_recv(socket, buf, ...)
>   do_something_else_with(buf);
>   ap_fwrite(r->connection->output_filters, obb, ...);
>   ap_fflush(r->connection->output_filters, obb, ...);
> }
>
> I am suffering from very occasional corruption of the bucket brigade which
> normally shows up as a corrupted pool pointer or a bogus bucket entry in
> thread #1 (for instance a SEGV in apr_brigade_length). Interestingly this
> is the relatively quiet input brigade which is only ever touched by the
> main apache thread. It's almost as if an allocator is not thread safe.

That's because it isn't unless you explicitly make it so (which no MPM does).

> However, I'm using a separate bucket allocator (see code above) and (at
> least in my code) a separate pool.

They're not really separate, the sub pool is created off r->pool. You
should probably use apr_pool_create_ex() here with parent=NULL and an
allocator that you created with apr_allocator_create() +
apr_allocator_mutex_set().

That won't solve all your problems though. Bucket brigades are not
thread safe, you will need something to synchronize on.

> Could the output filter chain somehow be using the allocator attached to
> the request (i.e. thread #1)? If so, how can I stop this? I can't run the
> ap_fwrite/ap_flush under a mutex and have the same mutex held during
> ap_get_brigade, as the latter blocks (and I can't see how to use the
> non-blocking version without spinning).
>
> [apologies for the partial dupe on the apache-users mailing list - I think
> I've got nearer the problem since then and this list seems more
> appropriate]
>
> --
> Alex Bligh