webserver: work around a potential crash bug in libmicrohttpd 0.9.x #2316

Merged
merged 1 commit into from Mar 1, 2013

Projects

None yet

2 participants

@Montellese
Member

As observed and investigated by @ulion, libmicrohttpd 0.9.x can cause crashes on specific platforms (e.g. iOS) when using a MHD_USE_SELECT_INTERNALLY in combination with a thread pool (MHD_OPTION_THREAD_POOL_SIZE) because of non- copyable mutex structures (see https://gnunet.org/bugs/view.php?id=2803 for the bug report).

This change uses MHD_USE_SELECT_INTERNALLY in combination with a thread pool for anything below 0.9.x (0x00090B01 specifically) and one thread per connection for anything above that to avoid the bug @ulion uncovered until it has been fixed.

Is this ok like this or do we want to test all platforms and enable/disable thread pooling based on the platform and the libmicrohttpd version being used?

@ulion
Collaborator
ulion commented Feb 27, 2013

I suggest temp deny thread pool until libmicrohttpd fixed, since it has issue after 0.9.12, we can not trust it's solid before 0.9.12 also. thread pool is complex than thread per connection anyway, so the safe way is to use one thread per connection in my opinion.

@Montellese
Member

TBH I'm opposed to moving away from thread pooling completely out of two reasons:

  • We've been using it with 0.4.6 on all platforms since 1.5 years on all platforms and there have never been any complaints.
  • Thread pooling is faster and less resource intensive than spawning a new thread for every connection. Before I switched to thread pooling I looked into some other webservers like Apache etc and they all usually use thread pooling.
@ulion
Collaborator
ulion commented Feb 27, 2013

well, since we will pump the libmicrohttpd version to newer ones and without thread pool, I agree with it.

@Montellese Montellese webserver: work around a potential crash bug in libmicrohttpd 0.9.x
As observed and investigated by ulion, libmicrohttpd 0.9.x can cause crashes
on specific platforms (e.g. iOS) when using a MHD_USE_SELECT_INTERNALLY in
combination with a thread pool (MHD_OPTION_THREAD_POOL_SIZE) because of non-
copyable mutex structures.
See https://gnunet.org/bugs/view.php?id=2803 for the bug report.
9ddbb2e
@Montellese
Member

OK I've moved the #ifdef around MHD_USE_SELECT_INTERNALLY and MHD_USE_THREAD_PER_CONNECTION into CWebServer::StartMHD() but still kept the functionality to be able to pass in other flags (I'm working on HTTPS support which will require this).

Is this ok to go in?

@ulion
Collaborator
ulion commented Mar 1, 2013

yes, it's good. and a good news the patch I commit to the up stream is already be accepted and in the svn.

@Montellese Montellese was assigned Mar 1, 2013
@Montellese Montellese merged commit 3a4d16f into xbmc:master Mar 1, 2013
@Montellese Montellese deleted the Montellese:libmicrohttpd_threading_workaround branch Mar 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment