Honza,

Looks pretty good; few comments:

corosync_shutdown_in_progress should be named something different such
as

coroipcs_shutdown_requested

why have a separate ipc_send_exitall vs ipc_exit?  Shouldn't ipc_exit
just execute the sending of messages (maybe by calling a static
coroipcs_ipc_send_exitall) to the clients and then pthread_join the
pending threads waiting for exit?  Also note this function does not
follow naming conventions, it should be coroipcs_exit_send or something
similar.

I am not convinced we need a separate extern interface for this
operation.

This line is not an ERROR but instead a NOTICE.  Also keep in mind
coroipcs is meant to be used by others so "Corosync" shouldn't be
directly used:
+               log_printf (LOGSYS_LEVEL_ERROR,
+                       "Corosync shutdown in progress. Do not accept
new connection\n");

Something like "Server shutdown in progress - not accepting new
connections"

Good work on the patch though.  See you after break.

Regards
-steve

On Thu, 2009-12-17 at 13:10 +0100, Jan Friesse wrote:
> Releated to https://bugzilla.redhat.com/show_bug.cgi?id=547511 bug.
> 
> It looks like MAIN problem was hidden in following posix definition:
> 
> Destroying  a  semaphore  that other processes or threads are currently 
> blocked on (in sem_wait(3)) produces undefined behavior.
> 
> 
> Sadly this is exactly what we were doing.
> 
> So patch is doing 2 things:
> - Don't accept new connections, when shutdown is in progress
> 
> - Send message about our shutdown -> every process waiting in sem_wait 
> is no longer waiting. In coroipcc, we will NEVER allow to sem_wait again 
> after receiving this message. Disconnect IPC and join to thread (so it 
> can handle its actions)
> 
> Patch looks pretty stable, most of the valgrind errors are gone and 
> corosync will shutdown properly every time in my tests.
> 
> Regards,
>    Honza
> 
> _______________________________________________
> Openais mailing list
> [email protected]
> https://lists.linux-foundation.org/mailman/listinfo/openais

_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais

Reply via email to