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 2013/06/12 10:46:59 UTC

Mutex protection of output bucket brigade

I think I've finally figured out what's going wrong in my module but am unsure what to do about it.

The module runs on apache 2.2.22 with mpm prefork. Occasionally I am seeing corruption of the output bucket brigade, primarily the ring pointers (link->next and link->prev) ending up with strange values.

Having spent some time reading the source, I believe that apache provides no protection to these pointers, and it's inherently unsafe for a bucket brigade to be used by more than one thread (even if you are careful with allocators), unless all callers provide their own mutex protection. As apache itself uses the output bucket brigade without mutex protection, the output bucket brigade can never be written to by other threads, and therefore ap_fwrite (to this brigade) can never be safely by any thread other than the main thread. First question: is this correct?

My module is currently structured as follows.

The main thread creates another thread for each request (the requests are long running websocket connections).

The main thread does the following:

  while (!done)
  {
    /* Blocking read */
    apr_brigade_create;
    ap_get_brigade;
    apr_brigade_flatten;

    /* Do stuff with the data */
    blocking_socket_write;
  }

The spawned thread does the following

  while (!done)
  {
     blocking_socket_read;
   
     /* do stuff with the data */
     ap_fwrite(output_bucket_brigade);

   }
    
Now, what I believe is happening is as follows. The blocking read in the main thread at some point calls select(), and does not only do a read, but also also a write of the data in the output bucket brigade. This removes a bucket from the ring. If this is happens at the same time as the ap_fwrite in the spawned thread adds something to the output ring, two threads will be accessing the ring pointers at once.

What I can't figure out is how to fix this.

I can't put in a mutex to protect the ring pointers, because the access to the ring pointers by apache is outside of my module.

I can't hold a mutex across the blocking read in the main thread, because otherwise my module won't be able to write data to the output bucket brigade whilst there is no input from the apache client; as the apache client may be waiting for data to be sent to it, this could cause deadlock.

And I can't obviously see how to do the read in a non-blocking way.

Any ideas?

-- 
Alex Bligh


Re: Mutex protection of output bucket brigade

Posted by Sorin Manolache <so...@gmail.com>.
On 2013-06-12 14:16, Alex Bligh wrote:
>
> But that aside, is it safe to call apwrite() from one thread whilst there's a read in the other?

I do not know for sure, but I suppose it is not safe if both the read 
and the write operate on the same brigade.

> I'm not calling ap_pass_brigade (at least not directly). I'm doing (roughly)
>
>    ap_filter_t *of = state->r->connection->output_filters;
>    ap_fwrite(of, state->obb, (const char *)header, pos); /* Header */
>
> IE I'm doing an fwrite to output filter list, using the bucket brigade I've just created. Is that OK?

I've checked the code of ap_fwrite. Apparently it buffers the data in 
the brigade. If the brigade is full, it calls ap_filter_flush, which, in 
turn, calls the ap_pass_brigade and clears the brigade after 
ap_pass_brigade has returned.

So not every call to ap_fwrite will push the data down the filter chain.

> Now I think I understand what happens with ap_fwrite (if that calls ap_pass_brigade), I'm wondering whether (say) the input filter of mod_ssl ever talks to its output filter.

I think it's impossible. The brigade is created by you in the spawned 
thread, so I don't see how the SSL decode in the main thread could 
access it. And it's the output brigade that is corrupted, not some 
brigade internal to mod_ssl. As I see it, the output brigade 
(state->obb) is not shared between the threads.

Do you get the same errors when you disable mod_ssl?

S





Re: Mutex protection of output bucket brigade

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

> "Normally" the output brigade is only written during the ap_rprintf/ap_fwrite and the like.

OK, so in my case, I run with an SSL filter and the request timeout stuff which also operates as a filter (let's ignore the latter for simplicity)

I think what you are saying is:

1. that the read from the socket on port 443 and the SSL decode of the client input is handled by the main thread, as is my processing of the decoded information.

2. that the apwrite from my second thread calls the SSL encode of the client output, and this writes to port 443.

I'm puzzled as to how this works without threading (perhaps the answer is 'it doesn't') as if both sets of I/O are blocking, there is a deadlock opportunity.

But that aside, is it safe to call apwrite() from one thread whilst there's a read in the other?

> There is no output brigade when the request_rec structure is created.

Sure. My second thread, dealing with data sent back to the client, is creating the output bucket brigade, with its own mutex and allocator.

    if (
        ( apr_thread_mutex_create(&oallocatormutex, APR_THREAD_MUTEX_UNNESTED, r->pool) == APR_SUCCESS) &&
        ( apr_allocator_create(&oallocator) == APR_SUCCESS) &&
        ( apr_allocator_mutex_set(oallocator, oallocatormutex), 1 ) &&
        ( apr_pool_create_ex(&opool, NULL, NULL, oallocator) == APR_SUCCESS) && /* WARNING: pool has no parent */
        ( NULL != (obucketallocator = apr_bucket_alloc_create(opool))) &&
        ( NULL != (obb = apr_brigade_create(opool, obucketallocator)))
        ) {

> The way to write something to the client is via ap_pass_brigade(r->output_filter, brigade). Typically the function that calls ap_pass_brigade creates the brigade first. So you write something to the brigade and you pass it downstream. The last filter writes its contents to the network. Be aware that there are filters that buffer the brigades and do not send them further down the chain unless the buffers are full.

I'm not calling ap_pass_brigade (at least not directly). I'm doing (roughly)

  ap_filter_t *of = state->r->connection->output_filters;
  ap_fwrite(of, state->obb, (const char *)header, pos); /* Header */

IE I'm doing an fwrite to output filter list, using the bucket brigade I've just created. Is that OK?

> If you created a handler that would just return OK without ever calling an ap_rprintf/ap_fwrite, the module would create no output brigade and the client would get no response.

Sure, because nothing would ever be sent to the filters.

> I think your hypothesis is unlikely. For it to hold, you'd need an input filter that is triggered when ap_get_brigade is called, that that input filter creates a brigade and stores it somewhere and that the spawned thread somehow writes to this stored brigade. So the ap_fwrite in the spawned thread would need to write to a brigade created in an input filter triggered by ap_get_brigade in the main thread.

Now I think I understand what happens with ap_fwrite (if that calls ap_pass_brigade), I'm wondering whether (say) the input filter of mod_ssl ever talks to its output filter.

> To verify this hypothesis, check which is the brigade whose pointers are corrupted. I mean, where is it referenced, in which apache module, in which apache filter. Then you can inspect the sources of that module to see if the brigade is shared between input and output.


It's always the output bucket brigade's pointers that are corrupted. Typically this causes a problem on destroy (as per the stack trace below). apr_brigade_cleanup in this instance never finishes (i.e. typing 'finish' in gdb on apr_bucket_destroy_noop works fine, but apr_brigade_cleanup will not 'finish' as it repeatedly calls apr_bucket_destroy_noop) as it goes around in an infinite loop. Sometimes we see SEGVs too. Typically the stack trace looks the same (i.e. it's ap_core_output_filter that is confused).

I suspect I may need to compile with DEBUG_BRIGADES or whatever it's called and see if I can get it to fail more often than once every 5 days on a torture test :-/

-- 
Alex Bligh

(gdb) bt
#0  0x00007f4f2bf082b0 in apr_bucket_destroy_noop () from /usr/lib/libaprutil-1.so.0
#1  0x00007f4f2bf072ac in apr_brigade_cleanup () from /usr/lib/libaprutil-1.so.0
#2  0x00007f4f2c5c38a5 in ap_core_output_filter ()
#3  0x00007f4f25210a3e in ?? () from /usr/lib/apache2/modules/mod_ssl.so
#4  0x00007f4f25210d4e in ?? () from /usr/lib/apache2/modules/mod_ssl.so
#5  0x00007f4f265802a9 in BIO_read () from /lib/x86_64-linux-gnu/libcrypto.so.1.0.0
#6  0x00007f4f2689bc54 in ?? () from /lib/x86_64-linux-gnu/libssl.so.1.0.0
#7  0x00007f4f2689cd7d in ?? () from /lib/x86_64-linux-gnu/libssl.so.1.0.0
#8  0x00007f4f2689a050 in ?? () from /lib/x86_64-linux-gnu/libssl.so.1.0.0
#9  0x00007f4f2520faf8 in ?? () from /usr/lib/apache2/modules/mod_ssl.so
#10 0x00007f4f25210810 in ?? () from /usr/lib/apache2/modules/mod_ssl.so
#11 0x00007f4f24df528a in ?? () from /usr/lib/apache2/modules/mod_websocket.so
#12 0x00007f4f24df6173 in ?? () from /usr/lib/apache2/modules/mod_websocket.so
#13 0x00007f4f24df688f in ?? () from /usr/lib/apache2/modules/mod_websocket.so
#14 0x00007f4f2c5c5508 in ap_run_handler ()
#15 0x00007f4f2c5c597e in ap_invoke_handler ()
#16 0x00007f4f2c5d5570 in ap_process_request ()
#17 0x00007f4f2c5d2398 in ?? ()
#18 0x00007f4f2c5cbfa8 in ap_run_process_connection ()
#19 0x00007f4f2c5da1d0 in ?? ()
#20 0x00007f4f2c5da93a in ?? ()
#21 0x00007f4f2c5db4e7 in ap_mpm_run ()
#22 0x00007f4f2c5b04a4 in main ()
(gdb) finish



Re: Mutex protection of output bucket brigade

Posted by Sorin Manolache <so...@gmail.com>.
On 2013-06-12 11:48, Alex Bligh wrote:
>
> On 12 Jun 2013, at 10:20, Sorin Manolache wrote:
>
>> If I understand correctly, the main thread belongs to your module, i.e. it is not a concise pseudo-code of the request processing in apache's code.
>
> The main thread is the (presumably single) thread of the prefork mpm process, created (I assume) by a fork() in apache's worker code.
>
> The pseudo code was what my long-running request handler does (after creating the other thread). IE, broadly speaking my request handler (the main thread if you like) does this:
>
>   apr_thread_create; /* create the spawned thread */
>   while (!done)
>    {
>      /* Blocking read */
>      apr_brigade_create;
>      ap_get_brigade;
>      apr_brigade_flatten;
>
>      /* Do stuff with the data */
>      blocking_socket_write;
>    }
>   apr_thread_join; /* wait until the spawned thread has executed */
>
>> I don't see where the output brigade appears in the main thread. I think this is critical, as the output_bucket_brigade is the data item shared between the two threads. ap_get_brigade triggers the execution of the chain of input filters. One of these input filters writes to the output brigade?
>
> ap_get_brigade is called with APR_BLOCK_READ.
>
> What I now /believe/ this does (because it's the only way data would actually get written) is write the post processed output bucket brigade to the client too. If this were not the case, it's difficult to see how a single threaded application would every write the output bucket brigade.
>
> Or are you saying the output bucket brigade is only actually written to the client during an ap_fwrite()? In which case are all the filters (primarily mod_ssl) guaranteed to be thread safe if a different thread is doing input from that doing output?

"Normally" the output brigade is only written during the 
ap_rprintf/ap_fwrite and the like.

There is no output brigade when the request_rec structure is created. 
The way to write something to the client is via 
ap_pass_brigade(r->output_filter, brigade). Typically the function that 
calls ap_pass_brigade creates the brigade first. So you write something 
to the brigade and you pass it downstream. The last filter writes its 
contents to the network. Be aware that there are filters that buffer the 
brigades and do not send them further down the chain unless the buffers 
are full.

If you created a handler that would just return OK without ever calling 
an ap_rprintf/ap_fwrite, the module would create no output brigade and 
the client would get no response.

I think your hypothesis is unlikely. For it to hold, you'd need an input 
filter that is triggered when ap_get_brigade is called, that that input 
filter creates a brigade and stores it somewhere and that the spawned 
thread somehow writes to this stored brigade. So the ap_fwrite in the 
spawned thread would need to write to a brigade created in an input 
filter triggered by ap_get_brigade in the main thread.

To verify this hypothesis, check which is the brigade whose pointers are 
corrupted. I mean, where is it referenced, in which apache module, in 
which apache filter. Then you can inspect the sources of that module to 
see if the brigade is shared between input and output.

S


Re: Mutex protection of output bucket brigade

Posted by Alex Bligh <al...@alex.org.uk>.
On 12 Jun 2013, at 10:20, Sorin Manolache wrote:

> If I understand correctly, the main thread belongs to your module, i.e. it is not a concise pseudo-code of the request processing in apache's code.

The main thread is the (presumably single) thread of the prefork mpm process, created (I assume) by a fork() in apache's worker code.

The pseudo code was what my long-running request handler does (after creating the other thread). IE, broadly speaking my request handler (the main thread if you like) does this:

 apr_thread_create; /* create the spawned thread */
 while (!done)
  {
    /* Blocking read */
    apr_brigade_create;
    ap_get_brigade;
    apr_brigade_flatten;

    /* Do stuff with the data */
    blocking_socket_write;
  }
 apr_thread_join; /* wait until the spawned thread has executed */

> I don't see where the output brigade appears in the main thread. I think this is critical, as the output_bucket_brigade is the data item shared between the two threads. ap_get_brigade triggers the execution of the chain of input filters. One of these input filters writes to the output brigade?

ap_get_brigade is called with APR_BLOCK_READ.

What I now /believe/ this does (because it's the only way data would actually get written) is write the post processed output bucket brigade to the client too. If this were not the case, it's difficult to see how a single threaded application would every write the output bucket brigade.

Or are you saying the output bucket brigade is only actually written to the client during an ap_fwrite()? In which case are all the filters (primarily mod_ssl) guaranteed to be thread safe if a different thread is doing input from that doing output?

-- 
Alex Bligh





Re: Mutex protection of output bucket brigade

Posted by Sorin Manolache <so...@gmail.com>.
On 2013-06-12 10:46, Alex Bligh wrote:
> I think I've finally figured out what's going wrong in my module but am unsure what to do about it.
>
> The module runs on apache 2.2.22 with mpm prefork. Occasionally I am seeing corruption of the output bucket brigade, primarily the ring pointers (link->next and link->prev) ending up with strange values.
>
> Having spent some time reading the source, I believe that apache provides no protection to these pointers, and it's inherently unsafe for a bucket brigade to be used by more than one thread (even if you are careful with allocators), unless all callers provide their own mutex protection. As apache itself uses the output bucket brigade without mutex protection, the output bucket brigade can never be written to by other threads, and therefore ap_fwrite (to this brigade) can never be safely by any thread other than the main thread. First question: is this correct?
>
> My module is currently structured as follows.
>
> The main thread creates another thread for each request (the requests are long running websocket connections).
>
> The main thread does the following:
>
>    while (!done)
>    {
>      /* Blocking read */
>      apr_brigade_create;
>      ap_get_brigade;
>      apr_brigade_flatten;
>
>      /* Do stuff with the data */
>      blocking_socket_write;
>    }
>
> The spawned thread does the following
>
>    while (!done)
>    {
>       blocking_socket_read;
>
>       /* do stuff with the data */
>       ap_fwrite(output_bucket_brigade);
>
>     }
>
> Now, what I believe is happening is as follows. The blocking read in the main thread at some point calls select(), and does not only do a read, but also also a write of the data in the output bucket brigade. This removes a bucket from the ring. If this is happens at the same time as the ap_fwrite in the spawned thread adds something to the output ring, two threads will be accessing the ring pointers at once.
>
> What I can't figure out is how to fix this.
>
> I can't put in a mutex to protect the ring pointers, because the access to the ring pointers by apache is outside of my module.
>
> I can't hold a mutex across the blocking read in the main thread, because otherwise my module won't be able to write data to the output bucket brigade whilst there is no input from the apache client; as the apache client may be waiting for data to be sent to it, this could cause deadlock.
>
> And I can't obviously see how to do the read in a non-blocking way.
>
> Any ideas?

If I understand correctly, the main thread belongs to your module, i.e. 
it is not a concise pseudo-code of the request processing in apache's code.

I don't see where the output brigade appears in the main thread. I think 
this is critical, as the output_bucket_brigade is the data item shared 
between the two threads. ap_get_brigade triggers the execution of the 
chain of input filters. One of these input filters writes to the output 
brigade?

Sorin