Skip to content

Conversation

@abligh
Copy link

@abligh abligh commented Sep 9, 2012

Please pull commit 04b57a6 from my fork. This fixes a bug whereby the output bucket brigade is created using the same pool and bucket allocator as the input thread is using, which causes a (very occasional) SEGV.

Diff is at:
abligh@04b57a6

@abligh
Copy link
Author

abligh commented Sep 15, 2012

Please also pull this one:
abligh@2d824f9

It would appear to need its own allocator as well as its own bucketbrigade allocator.

@disconnect
Copy link
Owner

I modified the code according to your first pull-request, but in a way that compiles under MSVC. As for the other "improve thread protection" changes, would you explain why you need another allocator and an allocator mutex?

@abligh
Copy link
Author

abligh commented Sep 23, 2012

Sure: You need another allocator (and pool) because the allocators themselves are not thread safe. Essentially they maintain a linked list of used and free memory, and there is no protection on the modification of that list. The default action on creating a pool is that the new pool inherits the allocator of its parent. This problematic when creating a new pool for a thread. The bucket brigade allocator is not really a new allocator (in the sense of a new thread safe allocator) because it gets its memory from the pool allocator attached to the pool you pass in. So to make allocation in the bucket brigade thread safe (which is what we need as the output bucket brigade is run in the created thread) we need to create a new pool with a different pool allocator (hence apr_allocator_create), and in that pool create a new bucket brigade allocator (apr_bucket_alloc_create).

I am not entirely sure why you need another allocator mutex, but the bug takes four or five hours to reproduce here, and that is what the apache-dev mailing list advised doing. For the sake of one line, and one call at set up time only, I put this in too.

Note the other change was to make the main thread mutex non-nested (rather than DEFAULT), as the default type is not always UNNESTED. Please see the warning here on apr_thread_mutex_create:
http://apr.apache.org/docs/apr/0.9/group__apr__thread__mutex.html#g927e99580a669f577fbcb6508814ff12

Alex Bligh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants