Use global mutex instead file lock to fix issues with threaded mpm's

[mturk]

Patch fixes persistent storage data corruption issues with threaded mpm's for Apache Httpd. Reason for that is because the persistent storage locking is based on SDBM file locking mechanism which works only across process boundaries thus making threaded mpm's like worker or event unsafe.
Instead file lock, the patch uses global mutex which is both process and thread safe.

We have observed this issue with
https://issues.jboss.org/browse/JWS-489

[dkopecek]

Any plans to merge or close this PR?

[zimmerle]

Hi @dkopecek,

Are you using this pull request? did you notice any performance impact in terms of throughput and latency?

@mturk: I am afraid that this global mutex will reduce a lot the performance, what is your thinking about it?

[zimmerle]

Just to confirm my suspicions, I have made a set of tests with and without this patch and the results are demonstrated bellow.

Results with the patch

Results without the patch

My suspicions is that the apr lock is way slower than the file lock used by the sdbm, therefore we have slower numbers in terms of request per second. The huge difference between the two locks is conceptual: SDBM lock fails while storing value in a locked collection while the apr's one [this patch] hold until the critical section is free of use to perform its operations.

In fact, this patch mitigate the problem, however at a cost of performance reduction.

• Documentation for SDBM:

  • https://ci.apache.org/projects/httpd/trunk/doxygen/group__APR__Util__DBM__SDBM.html#ga08c6121b4159ae86cec6e915e1e5d758

• SDBM open with shared lock:

  • https://github.com/SpiderLabs/ModSecurity/blob/v2/master/apache2/persist_dbm.c#L122
  • https://github.com/SpiderLabs/ModSecurity/blob/v2/master/apache2/persist_dbm.c#L205
  • https://github.com/SpiderLabs/ModSecurity/blob/v2/master/apache2/persist_dbm.c#L420
  • https://github.com/SpiderLabs/ModSecurity/blob/v2/master/apache2/persist_dbm.c#L611

I would suggest the creation of a compilation flag to define whenever a global lock should be used or not. Having this option will let the user to choose between collection-correctness or performance. In the end of the day the correctness could be handling by a 3rd party process.

[zimmerle]

This pull request was merged and the betterment provided by it will be available upon the utilization of the flag --enable-collection-global-lock during the ./configure.

Thank you @mturk.