Skip to content

Make output bucket brigades thread safe #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abligh wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Make output bucket brigades thread safe #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
04b57a6
Make output bucket brigade thread safe
abligh Sep 9, 2012
c683275
Add mod_websocket_vnc_proxy initial revision - compile with apxs2 -c -i
abligh Sep 11, 2012
bdbcffb
Add test directory for guacamole
abligh Sep 12, 2012
408b242
rect apparently takes few parameters
abligh Sep 12, 2012
f7681cf
Add handling of images - invoke with guactest -i
abligh Sep 13, 2012
21d7f66
force creation of build dir
abligh Sep 13, 2012
2d824f9
Improve thread protection
abligh Sep 15, 2012
0e282e9
Improve thread protection in mod_websocket_vnc_proxy
abligh Sep 15, 2012
a218aac
Allow absent key if no query is defined
abligh Oct 18, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions mod_websocket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,10 +472,31 @@ static void mod_websocket_data_framing(const WebSocketServer *server,
{
WebSocketState *state = server->state;
request_rec *r = state->r;
apr_bucket_brigade *obb =
apr_brigade_create(r->pool, r->connection->bucket_alloc);

if (obb != NULL) {
/* We cannot use the same bucket allocator for the ouput bucket brigade
* obb as the one associated with the connection (r->connection->bucket_alloc)
* because the same bucket allocator cannot be used in two different
* threads, and we use the connection bucket allocator in this
* thread - see docs on apr_bucket_alloc_create(). This results in
* occasional core dumps. So create our own bucket allocator and pool
* for output thread bucket brigade.
*/

apr_thread_mutex_t *oallocatormutex = NULL;
apr_allocator_t * oallocator = NULL;
apr_pool_t *opool = NULL;
apr_bucket_alloc_t *obucketallocator = NULL;
apr_bucket_brigade * obb = NULL;

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)))
) {

unsigned char block[BLOCK_DATA_SIZE];
apr_int64_t block_size;
apr_int64_t extension_bytes_remaining = 0;
Expand Down Expand Up @@ -808,7 +829,15 @@ static void mod_websocket_data_framing(const WebSocketServer *server,
/* We are done with the bucket brigade */
apr_thread_mutex_lock(state->mutex);
state->obb = NULL;
apr_brigade_destroy(obb);

/* Destroy the pool (which does not have a parent) manually
* which will destroy (inter alia) the bucket brigade and
* bucket brigade allocator
*/
apr_pool_destroy(opool);
apr_allocator_destroy(oallocator);
/* No need to destroy the mutex as that belongs to r->pool */

}
}

Expand Down Expand Up @@ -932,7 +961,7 @@ static int mod_websocket_method_handler(request_rec *r)
}

apr_thread_mutex_create(&state.mutex,
APR_THREAD_MUTEX_DEFAULT,
APR_THREAD_MUTEX_UNNESTED,
r->pool);
apr_thread_mutex_lock(state.mutex);

Expand Down
28 changes: 23 additions & 5 deletions mod_websocket_draft76.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,13 +529,12 @@ static int mod_websocket_method_handler(request_rec *r)
}
}

apr_thread_mutex_create(&state.mutex, APR_THREAD_MUTEX_DEFAULT, r->pool);
apr_thread_mutex_create(&state.mutex, APR_THREAD_MUTEX_UNNESTED, r->pool);
apr_thread_mutex_lock(state.mutex);

/* If the plugin supplies an on_connect function, it must return non-null on success */
if ((conf->plugin->on_connect == NULL) ||
((plugin_private = conf->plugin->on_connect(&server)) != NULL)) {
apr_bucket_brigade *obb;

/* Now that the connection has been established, disable the socket timeout */
apr_socket_timeout_set(ap_get_module_config(r->connection->conn_config, &core_module), -1);
Expand All @@ -548,9 +547,21 @@ static int mod_websocket_method_handler(request_rec *r)
ap_send_interim_response(r, 1);

/* Create the output bucket brigade */
obb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
apr_thread_mutex_t *oallocatormutex = NULL;
apr_allocator_t * oallocator = NULL;
apr_pool_t *opool = NULL;
apr_bucket_alloc_t *obucketallocator = NULL;
apr_bucket_brigade * obb;

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)))
) {

if (obb != NULL) {
unsigned char block[BLOCK_DATA_SIZE], *extended_data = NULL;
apr_off_t extended_data_offset = 0;
apr_size_t block_size, data_length = 0, extended_data_size = 0;
Expand Down Expand Up @@ -705,7 +716,14 @@ static int mod_websocket_method_handler(request_rec *r)
apr_thread_mutex_lock(state.mutex);
state.obb = NULL;

apr_brigade_destroy(obb);
/* Destroy the pool (which does not have a parent) manually
* which will destroy (inter alia) the bucket brigade and
* bucket brigade allocator
*/

apr_pool_destroy(opool);
apr_allocator_destroy(oallocator);
/* No need to destroy the mutex as that belongs to r->pool */
}
apr_thread_mutex_unlock(state.mutex);

Expand Down
Loading