Add locks to SafeFile, remove from readcb, introduce some LockGuards

src/FSCommon.cpp

@@ -95,20 +95,18 @@ bool copyFile(const char *from, const char *to)
 
 #elif defined(FSCom)
     // take SPI Lock
-    spiLock->lock();
+    concurrency::LockGuard g(spiLock);
     unsigned char cbuffer[16];
 
     File f1 = FSCom.open(from, FILE_O_READ);
     if (!f1) {
         LOG_ERROR("Failed to open source file %s", from);
-        spiLock->unlock();
         return false;
     }
 
     File f2 = FSCom.open(to, FILE_O_WRITE);
     if (!f2) {
         LOG_ERROR("Failed to open destination file %s", to);
-        spiLock->unlock();
         return false;
     }
 
@@ -120,7 +118,6 @@ bool copyFile(const char *from, const char *to)
     f2.flush();
     f2.close();
     f1.close();
-    spiLock->unlock();
     return true;
 #endif
 }

src/SafeFile.cpp

@@ -5,6 +5,7 @@
 // Only way to work on both esp32 and nrf52
 static File openFile(const char *filename, bool fullAtomic)
 {
+    concurrency::LockGuard g(spiLock);
     if (!fullAtomic)
         FSCom.remove(filename); // Nuke the old file to make space (ignore if it !exists)
 
@@ -53,14 +54,19 @@ bool SafeFile::close()
     if (!f)
         return false;
 
+    spiLock->lock();
     f.close();
+    spiLock->unlock();
     if (!testReadback())
         return false;
 
-    // brief window of risk here ;-)
-    if (fullAtomic && FSCom.exists(filename.c_str()) && !FSCom.remove(filename.c_str())) {
-        LOG_ERROR("Can't remove old pref file");
-        return false;
+    { // Scope for lock
+        concurrency::LockGuard g(spiLock);
+        // brief window of risk here ;-)
+        if (fullAtomic && FSCom.exists(filename.c_str()) && !FSCom.remove(filename.c_str())) {
+            LOG_ERROR("Can't remove old pref file");
+            return false;
+        }
     }
 
     String filenameTmp = filename;
@@ -76,6 +82,7 @@ bool SafeFile::close()
 /// Read our (closed) tempfile back in and compare the hash
 bool SafeFile::testReadback()
 {
+    concurrency::LockGuard g(spiLock);
     bool lfs_failed = lfs_assert_failed;
     lfs_assert_failed = false;
 

src/SafeFile.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "FSCommon.h"
+#include "SPILock.h"
 #include "configuration.h"
 
 #ifdef FSCom

src/mesh/NodeDB.cpp

@@ -839,7 +839,7 @@ LoadFileResult NodeDB::loadProto(const char *filename, size_t protoSize, size_t
 {
     LoadFileResult state = LoadFileResult::OTHER_FAILURE;
 #ifdef FSCom
-    spiLock->lock();
+    concurrency::LockGuard g(spiLock);
 
     auto f = FSCom.open(filename, FILE_O_READ);
 
@@ -859,7 +859,6 @@ LoadFileResult NodeDB::loadProto(const char *filename, size_t protoSize, size_t
     } else {
         LOG_ERROR("Could not open / read %s", filename);
     }
-    spiLock->unlock();
 #else
     LOG_ERROR("ERROR: Filesystem not implemented");
     state = LoadFileResult::NO_FILESYSTEM;
@@ -1026,7 +1025,6 @@ bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_
 {
     bool okay = false;
 #ifdef FSCom
-    spiLock->lock();
     auto f = SafeFile(filename, fullAtomic);
 
     LOG_INFO("Save %s", filename);
@@ -1039,7 +1037,6 @@ bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_
     }
 
     bool writeSucceeded = f.close();
-    spiLock->unlock();
 
     if (!okay || !writeSucceeded) {
         LOG_ERROR("Can't write prefs!");
@@ -1452,4 +1449,4 @@ void recordCriticalError(meshtastic_CriticalErrorCode code, uint32_t address, co
     LOG_ERROR("A critical failure occurred, portduino is exiting");
     exit(2);
 #endif
-}
+}

src/mesh/mesh-pb-constants.cpp

@@ -37,13 +37,11 @@ bool pb_decode_from_bytes(const uint8_t *srcbuf, size_t srcbufsize, const pb_msg
 bool readcb(pb_istream_t *stream, uint8_t *buf, size_t count)
 {
     bool status = false;
-    spiLock->lock();
     File *file = (File *)stream->state;
 
     if (buf == NULL) {
         while (count-- && file->read() != EOF)
             ;
-        spiLock->unlock();
         return count == 0;
     }
 
@@ -52,8 +50,6 @@ bool readcb(pb_istream_t *stream, uint8_t *buf, size_t count)
     if (file->available() == 0)
         stream->bytes_left = 0;
 
-    spiLock->unlock();
-
     return status;
 }
 

src/modules/RangeTestModule.cpp

@@ -210,16 +210,14 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp)
         LOG_DEBUG("gpsStatus->getDOP()          %d", gpsStatus->getDOP());
         LOG_DEBUG("-----------------------------------------");
     */
-    spiLock->lock();
+    concurrency::LockGuard g(spiLock);
     if (!FSBegin()) {
         LOG_DEBUG("An Error has occurred while mounting the filesystem");
-        spiLock->unlock();
         return 0;
     }
 
     if (FSCom.totalBytes() - FSCom.usedBytes() < 51200) {
         LOG_DEBUG("Filesystem doesn't have enough free space. Aborting write");
-        spiLock->unlock();
         return 0;
     }
 
@@ -232,7 +230,6 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp)
 
         if (!fileToWrite) {
             LOG_ERROR("There was an error opening the file for writing");
-            spiLock->unlock();
             return 0;
         }
 
@@ -252,7 +249,6 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp)
 
     if (!fileToAppend) {
         LOG_ERROR("There was an error opening the file for appending");
-        spiLock->unlock();
         return 0;
     }
 
@@ -295,7 +291,6 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp)
     fileToAppend.printf("\"%s\"\n", p.payload.bytes);
     fileToAppend.flush();
     fileToAppend.close();
-    spiLock->unlock();
 #endif
 
     return 1;

src/modules/Telemetry/Sensor/NAU7802Sensor.cpp

@@ -99,7 +99,6 @@ void NAU7802Sensor::tare()
 
 bool NAU7802Sensor::saveCalibrationData()
 {
-    spiLock->lock();
     auto file = SafeFile(nau7802ConfigFileName);
     nau7802config.zeroOffset = nau7802.getZeroOffset();
     nau7802config.calibrationFactor = nau7802.getCalibrationFactor();
@@ -113,6 +112,7 @@ bool NAU7802Sensor::saveCalibrationData()
     } else {
         okay = true;
     }
+    spiLock->lock();
     okay &= file.close();
     spiLock->unlock();
 
@@ -142,4 +142,4 @@ bool NAU7802Sensor::loadCalibrationData()
     return okay;
 }
 
-#endif
+#endif