From 049555ae5eb7c778e80f7b090530f2580aa84f3d Mon Sep 17 00:00:00 2001
From: yaworsky <yaworsky>
Date: Fri, 18 Nov 2005 09:37:37 +0000
Subject: Fixed race condition

---
 daemon/dest_file.c | 66 +++++++++++++++++++++---------------------------------
 1 file changed, 26 insertions(+), 40 deletions(-)

diff --git a/daemon/dest_file.c b/daemon/dest_file.c
index fb63195..7ede68c 100644
--- a/daemon/dest_file.c
+++ b/daemon/dest_file.c
@@ -236,10 +236,18 @@ static void flush_coalescer( struct file_writer* writer )
 
 /******************************************************************************
  * destroy_file_writer
+ *
+ * destination's critical section cs_file_writers must be held when calling
+ * this function
  */
 static void destroy_file_writer( struct file_writer* writer )
 {
     TRACE_ENTER( "%p\n", writer );
+    if( writer->destination )
+    {
+        struct dest_extra *extra = writer->destination->extra;
+        extra->file_writers = g_list_remove( extra->file_writers, writer );
+    }
     if( writer->fifo_event ) CloseHandle( writer->fifo_event );
     if( writer->shutdown_event ) CloseHandle( writer->shutdown_event );
     DeleteCriticalSection( &writer->fifo_cs );
@@ -251,6 +259,9 @@ static void destroy_file_writer( struct file_writer* writer )
 
 /******************************************************************************
  * create_file_writer
+ *
+ * destination's critical section cs_file_writers must be held when calling
+ * this function
  */
 static struct file_writer* create_file_writer( gchar* file_name,
                                                struct destination* destination )
@@ -304,27 +315,6 @@ error:
     return NULL;
 }
 
-/******************************************************************************
- * detach_writer_from_destination
- */
-static void detach_writer_from_destination( struct file_writer* writer )
-{
-    struct dest_extra *extra;
-
-    TRACE_ENTER( "%p\n", writer );
-    if( !writer->destination )
-    {
-        TRACE_LEAVE( "done; already detached\n" );
-        return;
-    }
-    extra = writer->destination->extra;
-    EnterCriticalSection( &extra->cs_file_writers );
-    extra->file_writers = g_list_remove( extra->file_writers, writer );
-    LeaveCriticalSection( &extra->cs_file_writers );
-    writer->destination = NULL;
-    TRACE_LEAVE( "done\n" );
-}
-
 /******************************************************************************
  * pop_messages_from_queue
  *
@@ -365,8 +355,10 @@ static void pop_messages_from_queue( struct file_writer* writer )
 static unsigned __stdcall writer_thread_proc( void* arg )
 {
     struct file_writer *writer = arg;
+    struct dest_extra *extra = writer->destination->extra;
     HANDLE wait_objects[2] = { writer->fifo_event, writer->shutdown_event };
     gchar *pathname;
+    gboolean inside_cs = FALSE;
  
     TRACE_ENTER( "writer=%p\n", writer );
 
@@ -402,35 +394,29 @@ static unsigned __stdcall writer_thread_proc( void* arg )
 
     for(;;)
     {
-        DWORD w = WaitForMultipleObjects( 2, wait_objects, FALSE, WRITER_KEEPALIVE_TIME );
-        if( WAIT_TIMEOUT == w )
+        switch( WaitForMultipleObjects( 2, wait_objects, FALSE, WRITER_KEEPALIVE_TIME ) )
         {
-            /* prepare to suicide; at this point a new message may be put into queue;
-               detach writer from destination and continue to write any pending messages */
-            detach_writer_from_destination( writer );
-            /* from now no new messages will be put into queue */
-            break;
-        }
-
-        if( WAIT_OBJECT_0 + 1 == w )
-            /* shutdown */
-            break;
+        case WAIT_TIMEOUT:
+        case WAIT_OBJECT_0 + 1:
+            /* no new messages should be put into the queue until we close the file */
+            EnterCriticalSection( &extra->cs_file_writers );
+            inside_cs = TRUE;
+            goto done;
 
-        if( WAIT_OBJECT_0 == w )
+        case WAIT_OBJECT_0:
             pop_messages_from_queue( writer );
+            break;
+        }
     }
 
 done:
-    /* FIXME: possible race condition
-       We detach writer from destination before close file.
-       So if a new message has arrived and detached writer is flushing its queue
-       then a new instance will can not open file.
-     */
-    detach_writer_from_destination( writer );
     pop_messages_from_queue( writer );
     flush_coalescer( writer );
     if( writer->fd != INVALID_HANDLE_VALUE ) CloseHandle( writer->fd );
+
     destroy_file_writer( writer );
+    if( inside_cs )
+        LeaveCriticalSection( &extra->cs_file_writers );
 
     purge_log_dirs();
 
-- 
cgit v1.2.3