You are viewing a plain text version of this content. The canonical link for it is here.
Posted to test-dev@httpd.apache.org by Sander Temme <sc...@covalent.net> on 2003/08/06 04:48:08 UTC
[PATCH] mod_specweb99.c: thread-safe lock
All,
Months ago, Madhu posted a patch to put thread-safe locks around the Post
logging in mod_specweb99... here's an alternative using apr_global_mutex
locks:
Index: mod_specweb99.c
===================================================================
RCS file: /home/cvs/httpd-test/specweb99/specweb99-2.0/mod_specweb99.c,v
retrieving revision 1.28
diff -u -r1.28 mod_specweb99.c
--- mod_specweb99.c 25 Jun 2003 18:51:11 -0000 1.28
+++ mod_specweb99.c 6 Aug 2003 01:29:29 -0000
@@ -84,6 +84,20 @@
#include <unistd.h>
#endif
+#if !defined(OS2) && !defined(WIN32) && !defined(BEOS) &&
!defined(NETWARE)
+#include "unixd.h"
+#define MOD_SPECWEB_SET_MUTEX_PERMS /* XXX Apache should define something
*/
+#endif
+
+/* Hardcoded lock file path for the Post log file mutex. Whether this file
+ * will actually be created depends on the default global mutex
implementation
+ * that APR was compiled with.
+ *
+ * Global mutex structure pointer for Post log mutex.
+ */
+#define LOCKFILENAME "/tmp/specweb99_lockfile"
+apr_global_mutex_t *log_mutex;
+
/* Note: version must be of the x.yy type - as it is
* send over the http protocol wire; where x and y
* are single 0-9 ascii digits :-). The name should
@@ -559,16 +573,57 @@
return (void *) _my;
}
+/* Get rid of the Post log mutex. This function is registered as
+ * cleanup for the pool that we are passed when the parent is
+ * initialized.
+ */
+
+static apr_status_t log_mutex_remove(void *data)
+{
+ apr_global_mutex_destroy(log_mutex);
+ log_mutex = NULL;
+ return(0);
+}
+
static int specweb99_module_init(apr_pool_t * p, apr_pool_t * plog,
apr_pool_t * ptemp, server_rec *s)
{
+ apr_status_t rv;
+
ap_add_version_component(p, NAME "/" VERSION);
ap_log_error(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, s,
NAME "/" VERSION " module: Compiled on %s at %s",
__DATE__,
__TIME__);
+
+ /* Create the Post log mutex, using the default global locking method
that
+ * APR was compiled with.
+ */
+ rv = apr_global_mutex_create(&log_mutex, LOCKFILENAME,
APR_LOCK_DEFAULT, p);
+ if (rv != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+ "mod_specweb99: Parent could not create Post log mutex "
+ "with file %s", LOCKFILENAME);
+ return HTTP_INTERNAL_SERVER_ERROR;
+ }
+
+ /* This is defined at the top of this file and causes the permissions
+ * function to not get called on platforms that don't require it.
+ */
+#ifdef MOD_SPECWEB_SET_MUTEX_PERMS
+ rv = unixd_set_global_mutex_perms(log_mutex);
+ if (rv != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+ "mod_specweb99: Parent could not set permissions "
+ "on Post log mutex; check User and Group directives");
+ return HTTP_INTERNAL_SERVER_ERROR;
+ }
+#endif /* MOD_SPECWEB_SET_MUTEX_PERMS */
+
+ apr_pool_cleanup_register(p, (void *)s, log_mutex_remove,
+ apr_pool_cleanup_null);
- return 0;
+ return OK;
}
static void specweb99_child_init(apr_pool_t * p, server_rec *s)
@@ -615,6 +670,13 @@
* concept of a docroot).
*/
};
+
+ /* Re-open the Post log mutex for this child. I assume this does the
+ * initialization of the intra-process part of the global mutex.
+ */
+ if (apr_global_mutex_child_init(&log_mutex, LOCKFILENAME, p) !=
APR_SUCCESS)
+ exit(APEXIT_CHILDFATAL);
+
} /* specweb99_child_init */
static int do_housekeeping(request_rec *r)
@@ -1160,7 +1222,7 @@
return HTTP_INTERNAL_SERVER_ERROR;
}
- if ((rv = _wlock(r->server, r, f, _my->log_path)) != APR_SUCCESS) {
+ if ((rv = apr_global_mutex_lock(log_mutex)) != APR_SUCCESS) {
returnHTMLPageWithMessage(r, "Failed to lock post.log file");
}
else {
@@ -1174,7 +1236,7 @@
}
}
- if ((rv2 = apr_file_unlock(f)) != APR_SUCCESS) {
+ if ((rv2 = apr_global_mutex_unlock(log_mutex)) != APR_SUCCESS) {
if (rv == APR_SUCCESS) {
rv = rv2;
ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,
(also attached to get around line-wrapping by the mailer).
This approach is also used in mod_rewrite. I know apr_global_mutexes are
advertised as potentially expensive, but in a hybrid situation you're going
to end up doing something like this, and apr_global_mutexes turn into
process locks when APR is thread-free or (not tested) on Win32.
Tested on Darwin under light load and Solaris under moderate load. Thoughts,
opinions, comments? Or shall I just commit the thing?
Thanks,
S.
--
Covalent Technologies sctemme@covalent.net
Engineering group Voice: (415) 856 4214
303 Second Street #375 South Fax: (415) 856 4210
San Francisco CA 94107
PGP Fingerprint: 7A8D B189 E871 80CB 9521 9320 C11E 7B47 964F 31D9
=======================================================
This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. Any unauthorized review,
use, disclosure or distribution is prohibited. If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message
=======================================================
Re: [PATCH] mod_specweb99.c: thread-safe lock
Posted by Greg Ames <gr...@apache.org>.
Greg Ames wrote:
> Sander Temme wrote:
>
>> Tested on Darwin under light load and Solaris under moderate load.
>> Thoughts,
>> opinions, comments? Or shall I just commit the thing?
>>
> I'll try it on the Linux PPC machine where I saw the post log
> flakyness and report back. I wouldn't have a problem if you just
> committed it but I should be able to beat it up heavily and report
> back this afternoon or tomorrow.
It looks a lot better on worker/Linux/PPC than it did before the
patch...a big reduction in complaints about the post log. There are
still a few, but why don't you go ahead and commit it? We can chase
down the remaining problems later.
Thanks again,
Greg
Re: [PATCH] mod_specweb99.c: thread-safe lock
Posted by Greg Ames <gr...@apache.org>.
Sander Temme wrote:
>All,
>
>Months ago, Madhu posted a patch to put thread-safe locks around the Post
>logging in mod_specweb99... here's an alternative using apr_global_mutex
>locks:
>
>
excellent! I was running specweb99 earlier this week and was seeing
post log bogosity with worker on Linux PPC. Maybe it's due to the
threads. Plus, I thought Madhu said that our current implementation is
slow on HP/UX too. Hopefully the APR default mutex will be a good
performer on our major platforms.
>This approach is also used in mod_rewrite. I know apr_global_mutexes are
>advertised as potentially expensive, but in a hybrid situation you're going
>to end up doing something like this, and apr_global_mutexes turn into
>process locks when APR is thread-free or (not tested) on Win32.
>
>
I think this approach is the Right Thing for now. Longer term, I would
like to use atomics a shared memory variable.
>Tested on Darwin under light load and Solaris under moderate load. Thoughts,
>opinions, comments? Or shall I just commit the thing?
>
I'll try it on the Linux PPC machine where I saw the post log flakyness and report back. I wouldn't have a problem if you just committed it but I should be able to beat it up heavily and report back this afternoon or tomorrow.
Thanks!
Greg