Hi List,
attched patch should fix race conditions in the accesslog and log module while
reopening from log files. Patch just add reader/writer lock and make sure
that writer are locked while log files will be reopened plus adds
log_shutdown(..) function in order to proper close log files.
Comments and votes are highly welcome!
--
Best regards / Mit besten Gr��en aus D�sseldorf
Dipl.-Ing.
Alexander Malysh
___________________________________________
Centrium GmbH
Vogelsanger Weg 80
40470 D�sseldorf
Fon: +49 (0211) 74 84 51 80
Fax: +49 (0211) 277 49 109
email: [EMAIL PROTECTED]
web: www.centrium.de
msn: [EMAIL PROTECTED]
icq: 98063111
___________________________________________
Please avoid sending me Word or PowerPoint attachments.
See http://www.fsf.org/philosophy/no-word-attachments.html
Index: gwlib/log.h
===================================================================
RCS file: /home/cvs/gateway/gwlib/log.h,v
retrieving revision 1.17
diff -a -u -r1.17 log.h
--- gwlib/log.h 4 Mar 2003 15:16:09 -0000 1.17
+++ gwlib/log.h 14 Oct 2003 13:26:17 -0000
@@ -30,6 +30,9 @@
/* Initialize the log file module */
void log_init();
+/* Shutdown the log file module */
+void log_shutdown();
+
/* Print a panicky error message and terminate the program with a failure.
* So, this function is called when there is no other choice than to exit
* immediately, with given reason
Index: gwlib/log.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/log.c,v
retrieving revision 1.38
diff -a -u -r1.38 log.c
--- gwlib/log.c 29 Aug 2003 17:29:58 -0000 1.38
+++ gwlib/log.c 14 Oct 2003 13:26:17 -0000
@@ -49,7 +49,7 @@
static int num_logfiles = 0;
-/*
+/*
* Mapping array between thread id and logfiles[] index.
* This is used for smsc specific logging.
*/
@@ -74,6 +74,11 @@
/*
+ * Reopen/rotate locking things.
+ */
+static List *writers = NULL;
+
+/*
* Syslog support.
*/
static int sysloglevel;
@@ -83,10 +88,10 @@
/*
* Make sure stderr is included in the list.
*/
-static void add_stderr(void)
+static void add_stderr(void)
{
int i;
-
+
for (i = 0; i < num_logfiles; ++i)
if (logfiles[i].file == stderr)
return;
@@ -105,14 +110,25 @@
for (i = 0; i <= THREADTABLE_SIZE; i++) {
thread_to[i] = 0;
}
+
+ add_stderr();
+
+ /* initialize rw lock */
+ writers = list_create();
+}
+
+void log_shutdown()
+{
+ log_close_all();
+ list_destroy(writers, NULL);
+ writers = NULL;
}
-void log_set_output_level(enum output_level level)
+void log_set_output_level(enum output_level level)
{
int i;
-
- add_stderr();
+
for (i = 0; i < num_logfiles; ++i) {
if (logfiles[i].file == stderr) {
logfiles[i].minimum_output_level = level;
@@ -121,10 +137,10 @@
}
}
-void log_set_log_level(enum output_level level)
+void log_set_log_level(enum output_level level)
{
int i;
-
+
/* change everything but stderr */
for (i = 0; i < num_logfiles; ++i) {
if (logfiles[i].file != stderr) {
@@ -135,7 +151,7 @@
}
-void log_set_syslog(const char *ident, int syslog_level)
+void log_set_syslog(const char *ident, int syslog_level)
{
if (ident == NULL)
dosyslog = 0;
@@ -148,18 +164,25 @@
}
-void log_reopen(void)
+void log_reopen(void)
{
- int i, j, found;
-
+ int i, j, found;
+
+ /*
+ * Writer lock.
+ */
+ list_lock(writers);
+ /* wait for writers complete */
+ list_consume(writers);
+
for (i = 0; i < num_logfiles; ++i) {
if (logfiles[i].file != stderr) {
found = 0;
- /*
+ /*
* Reverse seek for allready reopened logfile.
* If we find a previous file descriptor for the same file
- * name, then don't reopen that duplicate, but assign the
+ * name, then don't reopen that duplicate, but assign the
* file pointer to it.
*/
for (j = i-1; j >= 0 && found == 0; j--) {
@@ -178,40 +201,59 @@
logfiles[i].filename);
}
}
- }
+ }
+
+ /*
+ * Unlock writer.
+ */
+ list_unlock(writers);
}
-void log_close_all(void)
+void log_close_all(void)
{
+ /*
+ * Writer lock.
+ */
+ list_lock(writers);
+ /* wait for writers */
+ list_consume(writers);
+
while (num_logfiles > 0) {
--num_logfiles;
- if (logfiles[num_logfiles].file != stderr &&
+ if (logfiles[num_logfiles].file != stderr &&
logfiles[num_logfiles].file != NULL)
fclose(logfiles[num_logfiles].file);
logfiles[num_logfiles].file = NULL;
}
+
+ /*
+ * Unlock writer.
+ */
+ list_unlock(writers);
}
-int log_open(char *filename, int level, enum excl_state excl)
+int log_open(char *filename, int level, enum excl_state excl)
{
FILE *f = NULL;
int i;
- add_stderr();
+ if (writers == NULL)
+ writers = list_create();
+
if (num_logfiles == MAX_LOGFILES) {
- error(0, "Too many log files already open, not adding `%s'",
+ error(0, "Too many log files already open, not adding `%s'",
filename);
return -1;
}
-
+
if (strlen(filename) > FILENAME_MAX) {
error(0, "Log filename too long: `%s'.", filename);
return -1;
}
-
- /*
+
+ /*
* Check if the file is already opened for logging.
* If there is an open file, then assign the file descriptor
* that is already existing for this log file.
@@ -356,17 +398,20 @@
char buf[FORMAT_SIZE]; \
va_list args; \
\
- add_stderr(); \
format(buf, level, place, err, fmt); \
+ list_lock(writers); \
+ list_add_producer(writers); \
+ list_unlock(writers); \
for (i = 0; i < num_logfiles; ++i) { \
if (logfiles[i].exclusive == GW_NON_EXCL && \
- level >= logfiles[i].minimum_output_level && \
- logfiles[i].file != NULL) { \
- va_start(args, fmt); \
- output(logfiles[i].file, buf, args); \
- va_end(args); \
+ level >= logfiles[i].minimum_output_level && \
+ logfiles[i].file != NULL) { \
+ va_start(args, fmt); \
+ output(logfiles[i].file, buf, args); \
+ va_end(args); \
} \
} \
+ list_remove_producer(writers); \
if (dosyslog) { \
va_start(args, fmt); \
kannel_syslog(buf,args,level); \
@@ -379,25 +424,31 @@
char buf[FORMAT_SIZE]; \
va_list args; \
\
- add_stderr(); \
format(buf, level, place, err, fmt); \
- if (logfiles[e].exclusive == GW_EXCL && \
- level >= logfiles[e].minimum_output_level && \
- logfiles[e].file != NULL) { \
- va_start(args, fmt); \
- output(logfiles[e].file, buf, args); \
- va_end(args); \
- } \
+ list_lock(writers); \
+ list_add_producer(writers); \
+ list_unlock(writers); \
+ if (logfiles[e].exclusive == GW_EXCL && \
+ level >= logfiles[e].minimum_output_level && \
+ logfiles[e].file != NULL) { \
+ va_start(args, fmt); \
+ output(logfiles[e].file, buf, args); \
+ va_end(args); \
+ } \
+ list_remove_producer(writers); \
} while (0)
-void gw_panic(int err, const char *fmt, ...)
+void gw_panic(int err, const char *fmt, ...)
{
/*
* we don't want PANICs to spread accross smsc logs, so
* this will be always within the main core log.
*/
FUNCTION_GUTS(GW_PANIC, "");
+#ifdef SEGFAULT_PANIC
+ *((char*)0) = 0;
+#endif
exit(EXIT_FAILURE);
}
Index: gwlib/gwlib.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/gwlib.c,v
retrieving revision 1.14
diff -a -u -r1.14 gwlib.c
--- gwlib/gwlib.c 4 Mar 2003 15:16:09 -0000 1.14
+++ gwlib/gwlib.c 14 Oct 2003 13:26:17 -0000
@@ -44,5 +44,6 @@
gw_check_leaks();
gwmem_shutdown();
gwlib_protected_shutdown();
+ log_shutdown();
init = 0;
}
Index: gwlib/accesslog.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/accesslog.c,v
retrieving revision 1.2
diff -a -u -r1.2 accesslog.c
--- gwlib/accesslog.c 22 Jan 2001 12:23:43 -0000 1.2
+++ gwlib/accesslog.c 14 Oct 2003 13:26:17 -0000
@@ -21,28 +21,48 @@
static int use_localtime;
+/*
+ * Reopen/rotate lock.
+ */
+static List *writers = NULL;
+
void alog_reopen(void)
{
if (file == NULL)
return;
-
+
alog("Log ends");
+
+ list_lock(writers);
+ /* wait for writers to complete */
+ list_consume(writers);
+
fclose(file);
file = fopen(filename, "a");
+
+ list_unlock(writers);
+
if (file == NULL) {
error(errno, "Couldn't re-open access logfile `%s'.",
filename);
- } else
+ } else
alog("Log begins");
}
void alog_close(void)
{
+
if (file != NULL) {
alog("Log ends");
+ list_lock(writers);
+ /* wait for writers to complete */
+ list_consume(writers);
fclose(file);
file = NULL;
+ list_unlock(writers);
+ list_destroy(writers, NULL);
+ writers = NULL;
}
}
@@ -59,6 +79,10 @@
error(0, "Access Log filename too long: `%s', cannot open.", fname);
return;
}
+
+ if (writers == NULL)
+ writers = list_create();
+
f = fopen(fname, "a");
if (f == NULL) {
error(errno, "Couldn't open logfile `%s'.", fname);
@@ -120,8 +144,16 @@
buf = gw_malloc(FORMAT_SIZE + 1);
format(buf, fmt);
va_start(args, fmt);
+
+ list_lock(writers);
+ list_add_producer(writers);
+ list_unlock(writers);
+
vfprintf(file, buf, args);
fflush(file);
+
+ list_remove_producer(writers);
+
va_end(args);
gw_free(buf);
}
