Hi Lon
On Wed, 16 Apr 2008, Lon Hohberger wrote:
> Hi Fabio,
>
> I'd call this a 'pass 1' patch.
'pass 2' here :)
>
> * This makes your test program work.
> * This does not cause a regression with the existing logsys test cases
> in the openais trunk.
confirmed. I was able to run ccsd without any problem.
> It, however, dirties up the build system because it requires changing
> the linking order (logsys now needs -lpthread after it. For some
> reason, it didn't before (despite using other pthread APIs); someone
> probably knows why - as well as the correct way to fix it within the
> context of the openais build system.
I have been looking into this because my original patch was very close to
your but i don't have the definite answer other than linking with external
libraries should happen at the end in order to resolve all symbols.
So for example:
$(CC) $(LDFLAGS) $(EXEC_OBJS) $(EXEC_LIBS) -o aisexec
needs to turn into:
$(CC) $(EXEC_OBJS) $(EXEC_LIBS) -o aisexec $(LDFLAGS)
what still puzzles me is that we need to link all applications that uses
liblogsys to pthread and this was not necessary before but i am sure i am
overlooking at something. _maybe_ the fact that pthread_atfork interacts
directly with the application fork() invokation needs this explicit
linking.
> It would be *potentially* cleaner to make the child "cleanup" function
> an explicitly-called function, but probably less nice from a user API
> perspective. Basically, pthread_atfork() makes it a pretty simple thing
> since the user doesn't need to do anything in the child process (I think
> pthread_atfork was primarily designed for use in threaded APIs, anyway).
>
> Minimally, we needed to zap the mutexes and the thread indicators (since
> there's no threads in the child process... yet).
>
> Upon further investigation, I noticed a double-lock when doing
> _log_printf around the config mutex (_log_printf does a lock and so does
> log_printf_worker_fn, causing the child to hang on itself). To remedy
> this, I added a check for logsys_wthread_active (which will be 0 in the
> child process unless a full reinitialization is done) - but I'm not sure
> if this is the right thing to do or not.
It seems to work fine here. I guess in one way or another it is the right
thing to do.
Lon, thanks a lot for this patch.. Steven, can we please drive it into svn
at somepoing?
Thanks
Fabio
diff -urNad openais-0.82~/exec/Makefile openais-0.82/exec/Makefile
--- openais-0.82~/exec/Makefile 2008-04-17 07:27:02.000000000 +0200
+++ openais-0.82/exec/Makefile 2008-04-17 07:29:20.000000000 +0200
@@ -175,7 +175,7 @@
endif
aisexec: $(EXEC_OBJS) $(EXEC_LIBS)
- $(CC) $(LDFLAGS) $(EXEC_OBJS) $(EXEC_LIBS) -o aisexec
+ $(CC) $(EXEC_OBJS) $(EXEC_LIBS) -o aisexec $(LDFLAGS)
libtotem_pg.a: $(TOTEM_OBJS)
$(AR) -rc libtotem_pg.a $(TOTEM_OBJS)
diff -urNad openais-0.82~/exec/logsys.c openais-0.82/exec/logsys.c
--- openais-0.82~/exec/logsys.c 2008-04-17 07:27:02.000000000 +0200
+++ openais-0.82/exec/logsys.c 2008-04-17 07:27:23.000000000 +0200
@@ -255,7 +255,8 @@
{
struct log_data *log_data = (struct log_data *)work_item;
- pthread_mutex_lock (&logsys_config_mutex);
+ if (logsys_wthread_active)
+ pthread_mutex_lock (&logsys_config_mutex);
/*
* Output the log data
*/
@@ -273,7 +274,8 @@
&log_data->log_string[log_data->syslog_pos]);
}
free (log_data->log_string);
- pthread_mutex_unlock (&logsys_config_mutex);
+ if (logsys_wthread_active)
+ pthread_mutex_unlock (&logsys_config_mutex);
}
static void _log_printf (
@@ -467,6 +469,14 @@
pthread_mutex_unlock (&logsys_new_log_mutex);
}
+static void child_cleanup (void)
+{
+ memset(&log_thread_group, 0, sizeof(log_thread_group));
+ logsys_wthread_active = 0;
+ pthread_mutex_init(&logsys_config_mutex, NULL);
+ pthread_mutex_init(&logsys_new_log_mutex, NULL);
+}
+
int _logsys_wthread_create (void)
{
worker_thread_group_init (
@@ -481,6 +491,7 @@
logsys_flush();
atexit (logsys_atexit);
+ pthread_atfork(NULL, NULL, child_cleanup);
if (logsys_mode & LOG_MODE_OUTPUT_SYSLOG_THREADED && logsys_name !=
NULL) {
openlog (logsys_name, LOG_CONS|LOG_PID, logsys_facility);
diff -urNad openais-0.82~/test/Makefile openais-0.82/test/Makefile
--- openais-0.82~/test/Makefile 2008-04-17 07:27:02.000000000 +0200
+++ openais-0.82/test/Makefile 2008-04-17 07:29:02.000000000 +0200
@@ -161,13 +161,13 @@
$(CC) $(LDFLAGS) -o openais-cfgtool openais-cfgtool.o $(LIBS)
logsys_s: logsys_s.o logsys_s1.o logsys_s2.o ../exec/liblogsys.a
- $(CC) $(LDFLAGS) -o logsys_s logsys_s.o logsys_s1.o logsys_s2.o
../exec/liblogsys.a
+ $(CC) -o logsys_s logsys_s.o logsys_s1.o logsys_s2.o
../exec/liblogsys.a $(LDFLAGS)
logsys_t1: logsys_t1.o ../exec/liblogsys.a
- $(CC) $(LDFLAGS) -o logsys_t1 logsys_t1.o ../exec/liblogsys.a
+ $(CC) -o logsys_t1 logsys_t1.o ../exec/liblogsys.a $(LDFLAGS)
logsys_t2: logsys_t2.o ../exec/liblogsys.a
- $(CC) $(LDFLAGS) -o logsys_t2 logsys_t2.o ../exec/liblogsys.a
+ $(CC) -o logsys_t2 logsys_t2.o ../exec/liblogsys.a $(LDFLAGS)
clean:
rm -f *.o $(LIBRARIES) $(BINARIES)
--
I'm going to make him an offer he can't refuse.
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
