Merge pull request #652 from MartinMueller2003/main

Fixed a race condition in output config processing that causes a crash while processing the config file.
forkineye · Aug 22, 2023 · bb0baf2 · bb0baf2
2 parents f278eb0 + 0816257
commit bb0baf2
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 17 deletions.
diff --git a/ESPixelStick/src/WebMgr.cpp b/ESPixelStick/src/WebMgr.cpp
@@ -242,7 +242,6 @@ void c_WebMgr::init ()
             }
         });
 
-        // ping handler
     	webServer.on ("/files", HTTP_GET | HTTP_OPTIONS, [](AsyncWebServerRequest* request)
         {
             // DEBUG_V("files");
@@ -259,6 +258,24 @@ void c_WebMgr::init ()
             }
         });
 
+    	webServer.on ("/file/delete", HTTP_POST | HTTP_OPTIONS, [](AsyncWebServerRequest* request)
+        {
+            // DEBUG_V("/file/delete");
+            // DEBUG_V(String("URL: ") + request->url());
+            if(HTTP_OPTIONS == request->method())
+            {
+                request->send (200);
+            }
+            else
+            {
+                // DEBUG_V (String ("url: ") + String (request->url ()));
+                String filename = request->url ().substring (String ("/file/delete").length ());
+                // DEBUG_V (String ("filename: ") + String (filename));
+                FileMgr.DeleteSdFile(filename);
+                request->send (200);
+            }
+        });
+
         // JSON Config Handler
     	webServer.on ("/conf", HTTP_PUT | HTTP_POST | HTTP_OPTIONS,
         	[this](AsyncWebServerRequest* request)

diff --git a/ESPixelStick/src/output/OutputMgr.cpp b/ESPixelStick/src/output/OutputMgr.cpp
@@ -268,8 +268,7 @@ void c_OutputMgr::Begin ()
 
         // CreateNewConfig ();
 
-        // load up the configuration from the saved file. This also starts the drivers
-        // DEBUG_V();
+        // DEBUG_V("load up the configuration from the saved file. This also starts the drivers");
         LoadConfig();
 
         // CreateNewConfig ();
@@ -1003,6 +1002,9 @@ void c_OutputMgr::LoadConfig ()
 {
     // DEBUG_START;
 
+    ConfigLoadNeeded = false;
+    ConfigInProgress = true;
+
     // try to load and process the config file
     if (!FileMgr.LoadConfigFile(ConfigFileName, [this](DynamicJsonDocument &JsonConfigDoc)
         {
@@ -1032,6 +1034,8 @@ void c_OutputMgr::LoadConfig ()
         }
     }
 
+    ConfigInProgress = false;
+
     // DEBUG_END;
 
 } // LoadConfig
@@ -1290,11 +1294,10 @@ void c_OutputMgr::Poll()
     // do we need to save the current config?
     if (true == ConfigLoadNeeded)
     {
-        ConfigLoadNeeded = false;
         LoadConfig ();
     } // done need to save the current config
 
-    if (false == IsOutputPaused)
+    if ((false == IsOutputPaused) && (false == ConfigInProgress))
     {
         // //DEBUG_V();
         for (DriverInfo_t & OutputChannel : OutputChannelDrivers)
@@ -1318,23 +1321,30 @@ void c_OutputMgr::TaskPoll()
     // do we need to save the current config?
     if (true == ConfigLoadNeeded)
     {
-        ConfigLoadNeeded = false;
+        // DEBUG_V("Starting config processing");
         LoadConfig ();
+        // DEBUG_V("Done config processing");
     } // done need to save the current config
 
-    if (false == IsOutputPaused)
+    if ((false == IsOutputPaused) && (false == ConfigInProgress))
     {
         // PollCount ++;
         bool FoundAnActiveOutputChannel = false;
         // //DEBUG_V();
         for (DriverInfo_t & OutputChannel : OutputChannelDrivers)
         {
             // //DEBUG_V("Start a new channel");
-            uint32_t DelayInUs = OutputChannel.pOutputChannelDriver->Poll ();
-
-            if(DelayInUs)
+            if(nullptr != OutputChannel.pOutputChannelDriver)
+            {
+                uint32_t DelayInUs = OutputChannel.pOutputChannelDriver->Poll ();
+                if(DelayInUs)
+                {
+                    FoundAnActiveOutputChannel = true;
+                }
+            }
+            else
             {
-                FoundAnActiveOutputChannel = true;
+                // DEBUG_V(String("Missing pOutputChannelDriver for channel: ") + String(OutputChannel.DriverId));
             }
         }
 

diff --git a/ESPixelStick/src/output/OutputMgr.hpp b/ESPixelStick/src/output/OutputMgr.hpp
@@ -233,6 +233,7 @@ class c_OutputMgr
 
     bool HasBeenInitialized = false;
     bool ConfigLoadNeeded   = false;
+    bool ConfigInProgress   = false;
     bool IsOutputPaused     = false;
     bool BuildingNewConfig  = false;
 

diff --git a/html/script.js b/html/script.js
@@ -547,6 +547,8 @@ function RequestListOfFiles() {
         else
         {
             // console.info("SendCommand: Transaction complete");
+            clearTimeout(FseqFileListRequestTimer);
+            FseqFileListRequestTimer = null;
             ProcessGetFileListResponse(data);
             CompletedServerTransaction = true;
         }
@@ -619,17 +621,16 @@ function ProcessGetFileListResponse(JsonConfigData) {
 } // ProcessGetFileListResponse
 
 function RequestFileDeletion() {
-    let files = [];
 
     $('#FileManagementTable > tr').each(function (CurRowId) {
         if (true === $('#FileSelected_' + CurRowId).prop("checked")) {
-            let FileEntry = {};
-            FileEntry["name"] = $('#FileName_' + CurRowId).val().toString();
-            files.push(FileEntry);
+            let name = $('#FileName_' + CurRowId).val().toString();
+            console.info("delete file: " + name);
+            let Response = SendCommand('file/delete/' + name);
+            // console.info("delete Response: " + Response);
         }
     });
 
-    // TOBERESTORED wsEnqueue(JSON.stringify({ 'cmd': { 'delete': { 'files': files } } }));
     RequestListOfFiles();
 
 } // RequestFileDeletion

diff --git a/platformio.ini b/platformio.ini
@@ -106,7 +106,7 @@ build_flags = ${esp32.build_flags} -mtext-section-literals
 ; platform  = https://github.com/platformio/platform-espressif32.git#eb7eba4 ; unstable 6.3.2
 platform  = https://github.com/platformio/platform-espressif32.git#8100ac5 ; works 6.3.1
 platform_packages =
-;    framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.11 ;
+;    framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.11 ; uses a lot of ram
     framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.3 ;
 ;    framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.2 ; runs real slow
 ;    framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.1 ; Has general issues