From 39f53a4de0f977128120e220191ca2f90d8f6ae9 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sun, 27 Jan 2019 10:23:47 +0100 Subject: [PATCH] - took the delayed console command execution out of the thinker management. Doing this intermingled with the thinkers is highly unsafe because there are absolutely no guarantees about order of execution. Effectively it ran these commands right in the middle of the playsim which could cause all sorts of synchronization issues, because CCMDs are part of the UI, not the playsim. - pass a const string to AddCommandString. This function manipulated the input buffer, leading to all sorts of code contortions to make sure that the passed parameter is clean for that. This function will now create a copy of the passed parameter which it can manipulate without complicating its calling code. # Conflicts: # src/c_dispatch.cpp --- src/c_console.cpp | 14 +-- src/c_dispatch.cpp | 220 +++++++++++++++++----------------- src/c_dispatch.h | 5 +- src/d_main.cpp | 4 +- src/g_game.cpp | 4 +- src/g_inventory/a_weapons.cpp | 3 +- src/sound/oalsound.cpp | 3 +- 7 files changed, 121 insertions(+), 132 deletions(-) diff --git a/src/c_console.cpp b/src/c_console.cpp index caea2a741..8d2d1dfca 100644 --- a/src/c_console.cpp +++ b/src/c_console.cpp @@ -629,7 +629,7 @@ void C_DeinitConsole () while (cmd != NULL) { GameAtExit *next = cmd->Next; - AddCommandString (cmd->Command.LockBuffer()); + AddCommandString (cmd->Command); delete cmd; cmd = next; } @@ -1559,17 +1559,9 @@ static bool C_HandleKey (event_t *ev, FCommandBuffer &buffer) { // Work with a copy of command to avoid side effects caused by // exception raised during execution, like with 'error' CCMD. - // It's problematic to maintain FString's lock symmetry. - static TArray command; - const size_t length = buffer.Text.Len(); - - command.Resize(unsigned(length + 1)); - memcpy(&command[0], buffer.Text.GetChars(), length); - command[length] = '\0'; - + FString copy = buffer.Text; buffer.SetString(""); - - AddCommandString(&command[0]); + AddCommandString(copy); } TabbedLast = false; TabbedList = false; diff --git a/src/c_dispatch.cpp b/src/c_dispatch.cpp index b046f4467..b91ad83cc 100644 --- a/src/c_dispatch.cpp +++ b/src/c_dispatch.cpp @@ -59,38 +59,119 @@ // TYPES ------------------------------------------------------------------- -class DWaitingCommand : public DThinker +class UnsafeExecutionScope { - DECLARE_CLASS (DWaitingCommand, DThinker) + const bool wasEnabled; + public: - DWaitingCommand (const char *cmd, int tics, bool unsafe); - ~DWaitingCommand (); - void Serialize(FSerializer &arc); - void Tick (); + explicit UnsafeExecutionScope(const bool enable = true) + : wasEnabled(UnsafeExecutionContext) + { + UnsafeExecutionContext = enable; + } -private: - DWaitingCommand (); + ~UnsafeExecutionScope() + { + UnsafeExecutionContext = wasEnabled; + } +}; - char *Command; +class FDelayedCommand +{ +protected: + virtual bool Tick() = 0; + + friend class FDelayedCommandQueue; +}; + +class FWaitingCommand : public FDelayedCommand +{ +public: + FWaitingCommand(const char *cmd, int tics, bool unsafe) + : Command(cmd), TicsLeft(tics+1), IsUnsafe(unsafe) + {} + + bool Tick() override + { + if (--TicsLeft == 0) + { + UnsafeExecutionScope scope(IsUnsafe); + AddCommandString(Command); + return true; + } + return false; + } + + FString Command; int TicsLeft; bool IsUnsafe; }; -class DStoredCommand : public DThinker +class FStoredCommand : public FDelayedCommand { - DECLARE_CLASS (DStoredCommand, DThinker) public: - DStoredCommand (FConsoleCommand *com, const char *cmd); - ~DStoredCommand (); - void Tick (); + FStoredCommand(FConsoleCommand *com, const char *cmd) + : Command(com), Text(cmd) + {} + + bool Tick() override + { + if (Text.IsNotEmpty() && Command != nullptr) + { + FCommandLine args(Text); + Command->Run(args, players[consoleplayer].mo, 0); + } + return true; + } private: - DStoredCommand (); FConsoleCommand *Command; - char *Text; + FString Text; }; +class FDelayedCommandQueue +{ + TDeletingArray delayedCommands; +public: + void Run() + { + for (unsigned i = 0; i < delayedCommands.Size(); i++) + { + if (delayedCommands[i]->Tick()) + { + delete delayedCommands[i]; + delayedCommands.Delete(i); + i--; + } + } + } + + void Clear() + { + delayedCommands.DeleteAndClear(); + } + + void AddCommand(FDelayedCommand * cmd) + { + delayedCommands.Push(cmd); + } +}; + +static FDelayedCommandQueue delayedCommandQueue; + +void C_RunDelayedCommands() +{ + delayedCommandQueue.Run(); +} + +void C_ClearDelayedCommands() +{ + delayedCommandQueue.Clear(); +} + + + struct FActionMap { FButtonStatus *Button; @@ -127,22 +208,6 @@ FButtonStatus Button_Mlook, Button_Klook, Button_Use, Button_AltAttack, bool ParsingKeyConf, UnsafeExecutionContext; -class UnsafeExecutionScope -{ - const bool wasEnabled; - -public: - explicit UnsafeExecutionScope(const bool enable = true) - : wasEnabled(UnsafeExecutionContext) - { - UnsafeExecutionContext = enable; - } - - ~UnsafeExecutionScope() - { - UnsafeExecutionContext = wasEnabled; - } -}; // To add new actions, go to the console and type "key ". // This will give you the key value to use in the first column. Then @@ -205,79 +270,6 @@ static const char *KeyConfCommands[] = // CODE -------------------------------------------------------------------- -IMPLEMENT_CLASS(DWaitingCommand, false, false) - -void DWaitingCommand::Serialize(FSerializer &arc) -{ - Super::Serialize (arc); - arc("command", Command) - ("ticsleft", TicsLeft) - ("unsafe", IsUnsafe); -} - -DWaitingCommand::DWaitingCommand () -{ - Command = NULL; - TicsLeft = 1; - IsUnsafe = false; -} - -DWaitingCommand::DWaitingCommand (const char *cmd, int tics, bool unsafe) -{ - Command = copystring (cmd); - TicsLeft = tics+1; - IsUnsafe = unsafe; -} - -DWaitingCommand::~DWaitingCommand () -{ - if (Command != NULL) - { - delete[] Command; - } -} - -void DWaitingCommand::Tick () -{ - if (--TicsLeft == 0) - { - UnsafeExecutionScope scope(IsUnsafe); - AddCommandString (Command); - Destroy (); - } -} - -IMPLEMENT_CLASS(DStoredCommand, false, false) - -DStoredCommand::DStoredCommand () -{ - Text = NULL; - Destroy (); -} - -DStoredCommand::DStoredCommand (FConsoleCommand *command, const char *args) -{ - Command = command; - Text = copystring (args); -} - -DStoredCommand::~DStoredCommand () -{ - if (Text != NULL) - { - delete[] Text; - } -} - -void DStoredCommand::Tick () -{ - if (Text != NULL && Command != NULL) - { - FCommandLine args (Text); - Command->Run (args, players[consoleplayer].mo, 0); - } - Destroy (); -} static int ListActionCommands (const char *pattern) { @@ -656,7 +648,8 @@ void C_DoCommand (const char *cmd, int keynum) } else { - Create (com, beg); + auto cmd = new FStoredCommand(com, beg); + delayedCommandQueue.AddCommand(cmd); } } } @@ -696,8 +689,12 @@ DEFINE_ACTION_FUNCTION(DOptionMenuItemCommand, DoCommand) return 0; } -void AddCommandString (char *cmd, int keynum) +void AddCommandString (const char *text, int keynum) { + // Operate on a local copy instead of messing around with the data that's being passed in here. + TArray buffer(strlen(text) + 1, true); + memcpy(buffer.Data(), text, buffer.Size()); + char *cmd = buffer.Data(); char *brkpt; int more; @@ -752,7 +749,8 @@ void AddCommandString (char *cmd, int keynum) // Note that deferred commands lose track of which key // (if any) they were pressed from. *brkpt = ';'; - Create (brkpt, tics, UnsafeExecutionContext); + auto cmd = new FWaitingCommand(brkpt, tics, UnsafeExecutionContext); + delayedCommandQueue.AddCommand(cmd); } return; } @@ -1477,8 +1475,7 @@ void FConsoleAlias::Run (FCommandLine &args, AActor *who, int key) } bRunning = true; - AddCommandString (mycommand.LockBuffer(), key); - mycommand.UnlockBuffer(); + AddCommandString (mycommand, key); bRunning = false; if (m_Command[index].IsEmpty()) { // The alias is unchanged, so put the command back so it can be used again. @@ -1554,8 +1551,7 @@ void FExecList::ExecCommands() const { for (unsigned i = 0; i < Commands.Size(); ++i) { - AddCommandString(Commands[i].LockBuffer()); - Commands[i].UnlockBuffer(); + AddCommandString(Commands[i]); } } diff --git a/src/c_dispatch.h b/src/c_dispatch.h index 101bd8c63..e21e977d1 100644 --- a/src/c_dispatch.h +++ b/src/c_dispatch.h @@ -77,7 +77,10 @@ FExecList *C_ParseCmdLineParams(FExecList *exec); // and semicolon-separated commands. This function may modify the source // string, but the string will be restored to its original state before // returning. Therefore, commands passed must not be in read-only memory. -void AddCommandString (char *text, int keynum=0); +void AddCommandString (const char *text, int keynum=0); + +void C_RunDelayedCommands(); +void C_ClearDelayedCommands(); // Process a single console command. Does not handle wait. void C_DoCommand (const char *cmd, int keynum=0); diff --git a/src/d_main.cpp b/src/d_main.cpp index 82189c72a..b5aaced2d 100644 --- a/src/d_main.cpp +++ b/src/d_main.cpp @@ -2579,7 +2579,7 @@ void D_DoomMain (void) // [RH] Run any saved commands from the command line or autoexec.cfg now. gamestate = GS_FULLCONSOLE; Net_NewMakeTic (); - DThinker::RunThinkers (); + C_RunDelayedCommands(); gamestate = GS_STARTUP; if (!restart) @@ -2646,7 +2646,7 @@ void D_DoomMain (void) G_InitNew(startmap, false); if (StoredWarp.IsNotEmpty()) { - AddCommandString(StoredWarp.LockBuffer()); + AddCommandString(StoredWarp); StoredWarp = NULL; } } diff --git a/src/g_game.cpp b/src/g_game.cpp index 49551e4ad..56029333e 100644 --- a/src/g_game.cpp +++ b/src/g_game.cpp @@ -1021,9 +1021,8 @@ void G_Ticker () if (ToggleFullscreen) { - static char toggle_fullscreen[] = "toggle fullscreen"; ToggleFullscreen = false; - AddCommandString (toggle_fullscreen); + AddCommandString ("toggle fullscreen"); } // do things to change the game state @@ -1173,6 +1172,7 @@ void G_Ticker () // [ZZ] also tick the UI part of the events E_UiTick(); + C_RunDelayedCommands(); // do main actions switch (gamestate) diff --git a/src/g_inventory/a_weapons.cpp b/src/g_inventory/a_weapons.cpp index 8bb2aace3..fe5761222 100644 --- a/src/g_inventory/a_weapons.cpp +++ b/src/g_inventory/a_weapons.cpp @@ -748,8 +748,7 @@ void P_PlaybackKeyConfWeapons(FWeaponSlots *slots) PlayingKeyConf = slots; for (unsigned int i = 0; i < KeyConfWeapons.Size(); ++i) { - FString cmd(KeyConfWeapons[i]); - AddCommandString(cmd.LockBuffer()); + AddCommandString(KeyConfWeapons[i]); } PlayingKeyConf = nullptr; } diff --git a/src/sound/oalsound.cpp b/src/sound/oalsound.cpp index ed3098d9f..697a7a91f 100644 --- a/src/sound/oalsound.cpp +++ b/src/sound/oalsound.cpp @@ -2202,8 +2202,7 @@ void OpenALSoundRenderer::UpdateSounds() if(connected == ALC_FALSE) { Printf("Sound device disconnected; restarting...\n"); - static char snd_reset[] = "snd_reset"; - AddCommandString(snd_reset); + AddCommandString("snd_reset"); return; } }