Fix possible server memory leak.

Rework server shutdown procedure to make it safe to be run even if
server is not fully initalized yet. This fixes a possible memory leak
when ERR_DROP is thrown on initialization (due to missing game library,
for example).

5 files changed, 39 insertions, 37 deletions

diff --git a/src/mvd_client.c b/src/mvd_client.c
index 84c955a..c780ddc 100644
--- a/src/mvd_client.c
+++ b/src/mvd_client.c
@@ -381,7 +381,7 @@ int MVD_Frame( void ) {
     if( sv.state == ss_broadcast ) {
         unsigned delta = mvd_suspend_time->value * 60 * 1000;
 
-        if( !delta || !LIST_EMPTY( &svs.client_list ) ) {
+        if( !delta || !LIST_EMPTY( &sv_clientlist ) ) {
             prevtime = svs.realtime;
             if( !mvd_active ) {
                 Com_DPrintf( "Resuming MVD streams.\n" );
diff --git a/src/sv_ccmds.c b/src/sv_ccmds.c
index 918451c..70addbc 100644
--- a/src/sv_ccmds.c
+++ b/src/sv_ccmds.c
@@ -668,7 +668,7 @@ static void SV_Status_f( void ) {
         Com_Printf( "Current map: %s\n\n", sv.name );
     }
 
-    if( LIST_EMPTY( &svs.client_list ) ) {
+    if( LIST_EMPTY( &sv_clientlist ) ) {
         Com_Printf( "No UDP clients.\n" );
     } else {
         if( Cmd_Argc() > 1 ) {
@@ -901,7 +901,6 @@ static void SV_PickClient_f( void ) {
 SV_KillServer_f
 
 Kick everyone off, possibly in preparation for a new game
-
 ===============
 */
 static void SV_KillServer_f( void ) {
diff --git a/src/sv_init.c b/src/sv_init.c
index a6f7ec8..6d443a1 100644
--- a/src/sv_init.c
+++ b/src/sv_init.c
@@ -273,6 +273,7 @@ void SV_InitGame( qboolean ismvd ) {
 #endif
 
         CM_FreeMap( &sv.cm );
+        SV_FreeFile( sv.entitystring );
         memset( &sv, 0, sizeof( sv ) );
     }
 
@@ -360,8 +361,6 @@ void SV_InitGame( qboolean ismvd ) {
     // send heartbeat very soon
     svs.last_heartbeat = -(HEARTBEAT_SECONDS-5)*1000;
 
-    List_Init( &svs.client_list );
-
     for( i = 0; i < sv_maxclients->integer; i++ ) {
         client = svs.client_pool + i;
         entnum = i + 1;
diff --git a/src/sv_local.h b/src/sv_local.h
index e21d42e..c879a26 100644
--- a/src/sv_local.h
+++ b/src/sv_local.h
@@ -177,7 +177,7 @@ typedef struct {
 #define RATE_MESSAGES   10
 
 #define FOR_EACH_CLIENT( client ) \
-    LIST_FOR_EACH( client_t, client, &svs.client_list, entry )
+    LIST_FOR_EACH( client_t, client, &sv_clientlist, entry )
 
 #define PL_S2C(cl) (cl->frames_sent ? \
     (1.0f-(float)cl->frames_acked/cl->frames_sent)*100.0f : 0.0f)
@@ -372,7 +372,6 @@ typedef struct server_static_s {
     unsigned    realtime;           // always increasing, no clamping, etc
 
     client_t    *client_pool;   // [maxclients]
-    list_t      client_list;    // linked list of non-free clients
 
     unsigned        num_entities;   // maxclients*UPDATE_BACKUP*MAX_PACKET_ENTITIES
     unsigned        next_entity;    // next state to use
@@ -399,6 +398,7 @@ extern list_t      sv_blacklist;
 extern list_t      sv_cmdlist_connect;
 extern list_t      sv_cmdlist_begin;
 extern list_t      sv_filterlist;
+extern list_t      sv_clientlist; // linked list of non-free clients
 
 extern server_static_t     svs;        // persistant server info
 extern server_t            sv;         // local server
diff --git a/src/sv_main.c b/src/sv_main.c
index 466efe9..6bbf79a 100644
--- a/src/sv_main.c
+++ b/src/sv_main.c
@@ -28,6 +28,7 @@ LIST_DECL( sv_blacklist );
 LIST_DECL( sv_cmdlist_connect );
 LIST_DECL( sv_cmdlist_begin );
 LIST_DECL( sv_filterlist );
+LIST_DECL( sv_clientlist ); // linked list of non-free clients
 
 client_t    *sv_client;         // current client
 
@@ -106,7 +107,8 @@ void SV_RemoveClient( client_t *client ) {
         client->netchan = NULL;
     }
 
-    // unlink them from active client list
+    // unlink them from active client list, but don't clear the list entry
+    // itself to make code that traverses client list in a loop happy!
     List_Remove( &client->entry );
 
 #if USE_MVD_CLIENT
@@ -818,10 +820,8 @@ static void SVC_DirectConnect( void ) {
             }
 
             Com_DPrintf( "%s: reconnect\n", NET_AdrToString( &net_from ) );
-            newcl = cl;
-
-            // NOTE: it's safe to call SV_Remove since we exit the loop
             SV_RemoveClient( cl );
+            newcl = cl;
             break;
         }
     }
@@ -980,7 +980,7 @@ static void SVC_DirectConnect( void ) {
     }
 
     // add them to the linked list of connected clients
-    List_SeqAdd( &svs.client_list, &newcl->entry );
+    List_SeqAdd( &sv_clientlist, &newcl->entry );
 
     Com_DPrintf( "Going from cs_free to cs_assigned for %s\n", newcl->name );
     newcl->state = cs_assigned;
@@ -1452,7 +1452,7 @@ static inline qboolean check_paused( void ) {
 #if USE_MVD_CLIENT
         LIST_EMPTY( &mvd_gtv_list ) &&
 #endif
-        LIST_SINGLE( &svs.client_list ) )
+        LIST_SINGLE( &sv_clientlist ) )
     {
         if( !sv_paused->integer ) {
             Cvar_Set( "sv_paused", "1" );
@@ -1577,6 +1577,11 @@ Informs all masters that this server is going down
 static void SV_MasterShutdown( void ) {
     master_t *m;
 
+    // reset ack times
+    FOR_EACH_MASTER( m ) {
+        m->last_ack = 0;
+    }
+
     if( !Com_IsDedicated() )
         return;        // only dedicated servers send heartbeats
 
@@ -1981,20 +1986,28 @@ Used by SV_Shutdown to send a final message to all
 connected clients before the server goes down. The messages are sent
 immediately, not just stuck on the outgoing message list, because the
 server is going to totally exit after returning from this function.
+
+Also resposible for freeing all clients.
 ==================
 */
-static void SV_FinalMessage( const char *message, int cmd ) {
+static void SV_FinalMessage( const char *message, error_type_t type ) {
     client_t    *client;
     netchan_t   *netchan;
     int         i;
 
+    if( LIST_EMPTY( &sv_clientlist ) )
+        return;
+
     if( message ) {
         MSG_WriteByte( svc_print );
         MSG_WriteByte( PRINT_HIGH );
         MSG_WriteString( message );
     }
 
-    MSG_WriteByte( cmd );
+    if( type == ERR_RECONNECT )
+        MSG_WriteByte( svc_reconnect );
+    else
+        MSG_WriteByte( svc_disconnect );
 
     // send it twice
     // stagger the packets to crutch operating system limited buffers
@@ -2020,6 +2033,8 @@ static void SV_FinalMessage( const char *message, int cmd ) {
         }
         SV_RemoveClient( client );
     }
+
+    List_Init( &sv_clientlist );
 }
 
 
@@ -2027,37 +2042,28 @@ static void SV_FinalMessage( const char *message, int cmd ) {
 ================
 SV_Shutdown
 
-Called when each game quits, from Com_Quit or Com_Error
+Called when each game quits, from Com_Quit or Com_Error.
+Should be safe to call even if server is not fully initalized yet.
 ================
 */
 void SV_Shutdown( const char *finalmsg, error_type_t type ) {
-    master_t *m;
-
-    Cvar_Set( "sv_running", "0" );
-    Cvar_Set( "sv_paused", "0" );
-
-    if( !svs.initialized ) {
 #if USE_MVD_CLIENT
-        MVD_Shutdown(); // make sure MVD client is down
-#endif
-        return;
+    if( ge != &mvd_ge ) {
+        // shutdown MVD client now if not running the built-in MVD game module,
+        // otherwise SV_ShutdownGameProgs will take care of this
+        MVD_Shutdown();
     }
+#endif
 
 #if USE_AC_SERVER
     AC_Disconnect();
 #endif
 
 #if USE_MVD_SERVER
-    // shutdown MVD server
     SV_MvdShutdown( type );
 #endif
 
-    if( type == ERR_RECONNECT ) {
-        SV_FinalMessage( finalmsg, svc_reconnect );
-    } else {
-        SV_FinalMessage( finalmsg, svc_disconnect );
-    }
-
+    SV_FinalMessage( finalmsg, type );
     SV_MasterShutdown();
     SV_ShutdownGameProgs();
 
@@ -2074,17 +2080,15 @@ void SV_Shutdown( const char *finalmsg, error_type_t type ) {
 #endif
     memset( &svs, 0, sizeof( svs ) );
 
-    // reset masters
-    FOR_EACH_MASTER( m ) {
-        m->last_ack = 0;
-    }
-
     // reset rate limits
     init_rate_limits();
 
     sv_client = NULL;
     sv_player = NULL;
 
+    Cvar_Set( "sv_running", "0" );
+    Cvar_Set( "sv_paused", "0" );
+
 #if USE_SYSCON
     SV_SetConsoleTitle();
 #endif