Fix 8 netdemo bugs found in code review

1. File handle leak: SVD_Play_f opened file twice, first handle leaked.
   Fix: memset demo state before opening.

2. svdemo_stop now handles both recording and playback via SVD_Stop_f.
   Playback stop disconnects client to return to menu.

3. Zombie client timeout: skip SV_CheckTimeouts during playback so
   reserved player slots aren't freed.

4. Buffer overflow: increase entity buffer to MAX_GENTITIES*300 and
   playerState buffer to MAX_CLIENTS*600 for worst-case first frame.
   Made static to avoid stack overflow.

5. svs.time jump: don't overwrite svs.time with recorded time.
   Server time advances normally, avoiding timeout/heartbeat issues.

6. map_restart: SVD_ResetDeltaState clears entity/player delta state
   so next frame writes full states, preventing corrupt deltas.

7. Demo end and manual stop both disconnect the client.

8. SV_Shutdown cleans up active recording/playback.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
serge_shubin 2026-03-23 05:03:33 +08:00
parent b48a0575f1
commit a1167ff398
4 changed files with 48 additions and 13 deletions

View file

@ -347,8 +347,10 @@ void BotImport_DebugPolygonDelete(int id);
void SVD_Record_f( void ); void SVD_Record_f( void );
void SVD_StopRecord_f( void ); void SVD_StopRecord_f( void );
void SVD_RecordFrame( void ); void SVD_RecordFrame( void );
void SVD_ResetDeltaState( void );
void SVD_Play_f( void ); void SVD_Play_f( void );
void SVD_StopPlay_f( void ); void SVD_StopPlay_f( void );
void SVD_Stop_f( void );
qboolean SVD_PlaybackFrame( void ); qboolean SVD_PlaybackFrame( void );
qboolean SVD_IsRecording( void ); qboolean SVD_IsRecording( void );
qboolean SVD_IsPlaying( void ); qboolean SVD_IsPlaying( void );

View file

@ -280,6 +280,9 @@ static void SV_MapRestart_f( void ) {
sv.state = SS_GAME; sv.state = SS_GAME;
sv.restarting = qfalse; sv.restarting = qfalse;
// reset demo delta state so next frame writes full entities
SVD_ResetDeltaState();
// connect and begin all the clients // connect and begin all the clients
for (i=0 ; i<sv_maxclients->integer ; i++) { for (i=0 ; i<sv_maxclients->integer ; i++) {
client = &svs.clients[i]; client = &svs.clients[i];
@ -739,7 +742,7 @@ void SV_AddOperatorCommands( void ) {
// server-side demo recording/playback // server-side demo recording/playback
Cmd_AddCommand ("svdemo_record", SVD_Record_f); Cmd_AddCommand ("svdemo_record", SVD_Record_f);
Cmd_AddCommand ("svdemo_stop", SVD_StopRecord_f); Cmd_AddCommand ("svdemo_stop", SVD_Stop_f);
Cmd_AddCommand ("svdemo_play", SVD_Play_f); Cmd_AddCommand ("svdemo_play", SVD_Play_f);
} }

View file

@ -850,8 +850,10 @@ void SV_Frame( int msec ) {
time_game = Sys_Milliseconds () - startTime; time_game = Sys_Milliseconds () - startTime;
} }
// check timeouts // check timeouts (skip during demo playback — zombie slots would be freed)
SV_CheckTimeouts(); if ( !SVD_IsPlaying() ) {
SV_CheckTimeouts();
}
// send messages back to the clients // send messages back to the clients
SV_SendClientMessages(); SV_SendClientMessages();

View file

@ -175,7 +175,7 @@ static void SVD_WriteFrame( fileHandle_t f ) {
sharedEntity_t *ent; sharedEntity_t *ent;
short numChanges; short numChanges;
msg_t msg; msg_t msg;
byte msgBuf[MAX_GENTITIES * 64]; // delta-compressed entities are small static byte msgBuf[MAX_GENTITIES * 300]; // worst case: all entities full write from baseline
int msgLen; int msgLen;
SVD_WriteInt( f, svs.time ); SVD_WriteInt( f, svs.time );
@ -253,7 +253,7 @@ static void SVD_WriteFrame( fileHandle_t f ) {
// write player states (delta compressed) // write player states (delta compressed)
{ {
msg_t psmsg; msg_t psmsg;
byte psBuf[MAX_CLIENTS * 256]; static byte psBuf[MAX_CLIENTS * 600]; // worst case: full playerState from baseline
int psCount = 0; int psCount = 0;
MSG_Init( &psmsg, psBuf, sizeof(psBuf) ); MSG_Init( &psmsg, psBuf, sizeof(psBuf) );
@ -387,6 +387,18 @@ void SVD_StopRecord_f( void ) {
/* /*
Called from SV_Frame() after the game has run its frame. Called from SV_Frame() after the game has run its frame.
*/ */
/*
Reset delta compression state. Call on map_restart so the next
recorded frame writes full entity/player states from baseline.
*/
void SVD_ResetDeltaState( void ) {
if ( !demo.recording ) {
return;
}
Com_Memset( demo.prevEntities, 0, sizeof(demo.prevEntities) );
Com_Memset( demo.prevPlayers, 0, sizeof(demo.prevPlayers) );
}
void SVD_RecordFrame( void ) { void SVD_RecordFrame( void ) {
if ( !demo.recording ) { if ( !demo.recording ) {
return; return;
@ -472,7 +484,7 @@ static qboolean SVD_ReadFrame( fileHandle_t f ) {
int i, entNum, msgLen; int i, entNum, msgLen;
sharedEntity_t *ent; sharedEntity_t *ent;
msg_t msg; msg_t msg;
byte msgBuf[MAX_GENTITIES * 64]; static byte msgBuf[MAX_GENTITIES * 300];
entityState_t newEs; entityState_t newEs;
serverTime = SVD_ReadInt( f ); serverTime = SVD_ReadInt( f );
@ -480,7 +492,9 @@ static qboolean SVD_ReadFrame( fileHandle_t f ) {
return qfalse; // end of demo return qfalse; // end of demo
} }
svs.time = serverTime; // don't overwrite svs.time — it advances normally via SV_Frame.
// the recorded serverTime is consumed but not applied, avoiding
// time jumps that break client timeouts and heartbeats.
numEnts = SVD_ReadShort( f ); numEnts = SVD_ReadShort( f );
// read compressed message // read compressed message
@ -588,7 +602,7 @@ static qboolean SVD_ReadFrame( fileHandle_t f ) {
// read player states (delta compressed) // read player states (delta compressed)
{ {
msg_t psmsg; msg_t psmsg;
byte psBuf[MAX_CLIENTS * 256]; static byte psBuf[MAX_CLIENTS * 600]; // worst case: full playerState from baseline
int psMsgLen; int psMsgLen;
psMsgLen = SVD_ReadInt( f ); psMsgLen = SVD_ReadInt( f );
@ -679,17 +693,14 @@ void SVD_Play_f( void ) {
Com_sprintf( name, sizeof(name), "svdemos/%s.svdm", s ); Com_sprintf( name, sizeof(name), "svdemos/%s.svdm", s );
memset( &demo, 0, sizeof(demo) );
len = FS_FOpenFileRead( name, &demo.playFile, qtrue ); len = FS_FOpenFileRead( name, &demo.playFile, qtrue );
if ( !demo.playFile || len <= 0 ) { if ( !demo.playFile || len <= 0 ) {
Com_Printf( "ERROR: couldn't open %s.\n", name ); Com_Printf( "ERROR: couldn't open %s.\n", name );
return; return;
} }
// read the header (populates demo.playMapName etc.)
memset( &demo, 0, sizeof(demo) );
demo.playFile = 0; // re-open after memset
len = FS_FOpenFileRead( name, &demo.playFile, qtrue );
if ( !SVD_ReadHeader( demo.playFile ) ) { if ( !SVD_ReadHeader( demo.playFile ) ) {
FS_FCloseFile( demo.playFile ); FS_FCloseFile( demo.playFile );
demo.playFile = 0; demo.playFile = 0;
@ -770,6 +781,22 @@ void SVD_StopPlay_f( void ) {
SVD_CleanupPlayback(); SVD_CleanupPlayback();
Com_Printf( "Server demo playback stopped.\n" ); Com_Printf( "Server demo playback stopped.\n" );
// disconnect to return to main menu
Cbuf_ExecuteText( EXEC_APPEND, "disconnect\n" );
}
/*
Unified stop command: stops recording or playback, whichever is active.
*/
void SVD_Stop_f( void ) {
if ( demo.recording ) {
SVD_StopRecord_f();
} else if ( demo.playing ) {
SVD_StopPlay_f();
} else {
Com_Printf( "Not recording or playing a server demo.\n" );
}
} }
/* /*
@ -790,6 +817,7 @@ qboolean SVD_PlaybackFrame( void ) {
if ( !SVD_ReadFrame( demo.playFile ) ) { if ( !SVD_ReadFrame( demo.playFile ) ) {
Com_Printf( "Server demo playback finished.\n" ); Com_Printf( "Server demo playback finished.\n" );
SVD_CleanupPlayback(); SVD_CleanupPlayback();
Cbuf_ExecuteText( EXEC_APPEND, "disconnect\n" );
return qfalse; return qfalse;
} }