Bug fix/always remove lock file (#6314)

* always remove LOCK file

* disable useless messages about environment for upgrade check and password reset

arangod/RestServer/CheckVersionFeature.cpp

@@ -92,6 +92,10 @@ void CheckVersionFeature::validateOptions(
   DatabaseFeature* databaseFeature =
       ApplicationServer::getFeature<DatabaseFeature>("Database");
   databaseFeature->enableCheckVersion();
+
+  // we can turn off all warnings about environment here, because they
+  // wil show up on a regular start later anyway
+  ApplicationServer::disableFeatures({"Environment"});
 }
 
 void CheckVersionFeature::start() {

arangod/RestServer/InitDatabaseFeature.cpp

@@ -77,6 +77,10 @@ void InitDatabaseFeature::validateOptions(
   if (_initDatabase || _restoreAdmin) {
     ApplicationServer::forceDisableFeatures(_nonServerFeatures);
     ServerState::instance()->setRole(ServerState::ROLE_SINGLE);
+  
+    // we can turn off all warnings about environment here, because they
+    // wil show up on a regular start later anyway
+    ApplicationServer::disableFeatures({"Environment"});
   }
 }
 

lib/ApplicationFeatures/EnvironmentFeature.cpp

@@ -41,7 +41,7 @@ EnvironmentFeature::EnvironmentFeature(
     application_features::ApplicationServer& server
 )
     : ApplicationFeature(server, "Environment") {
-  setOptional(false);
+  setOptional(true);
   startsAfter("GreetingsPhase");
   startsAfter("MaxMapCount");
 }

lib/Basics/CleanupFunctions.h

@@ -32,7 +32,7 @@ namespace basics {
 
 /**
  * @brief A class to manage and handle cleanup functions on shutdown.
- * Thread save execution. It is meant to collect functions that should
+ * Thread-safe execution. It is meant to collect functions that should
  * be executed on every shutdown we can react to.
  * e.g. FATAL errors
  */

lib/Basics/application-exit.cpp

@@ -22,12 +22,16 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "Basics/Common.h"
+#include "Basics/CleanupFunctions.h"
 
 static void defaultExitFunction(int, void*);
 
 TRI_ExitFunction_t TRI_EXIT_FUNCTION = defaultExitFunction;
 
-void defaultExitFunction(int exitCode, void* /*data*/) { _exit(exitCode); }
+void defaultExitFunction(int exitCode, void* /*data*/) { 
+  arangodb::basics::CleanupFunctions::run(exitCode, nullptr); 
+  _exit(exitCode); 
+}
 
 void TRI_Application_Exit_SetExit(TRI_ExitFunction_t exitFunction) {
   if (exitFunction != nullptr) {

lib/Basics/files.cpp

@@ -51,40 +51,51 @@
 using namespace arangodb::basics;
 using namespace arangodb;
 
-////////////////////////////////////////////////////////////////////////////////
-/// @brief read buffer size (used for bulk file reading)
-////////////////////////////////////////////////////////////////////////////////
+namespace {
 
-#define READBUFFER_SIZE 8192
-
-////////////////////////////////////////////////////////////////////////////////
-/// @brief a static buffer of zeros, used to initialize files
-////////////////////////////////////////////////////////////////////////////////
-
-static char NullBuffer[4096];
-
-////////////////////////////////////////////////////////////////////////////////
-/// @brief already initialized
-////////////////////////////////////////////////////////////////////////////////
-
-static bool Initialized = false;
-
-////////////////////////////////////////////////////////////////////////////////
 /// @brief names of blocking files
-////////////////////////////////////////////////////////////////////////////////
-
 #ifdef TRI_HAVE_WIN32_FILE_LOCKING
 std::vector<std::pair<std::string, HANDLE>> OpenedFiles;
 #else
 std::vector<std::pair<std::string, int>> OpenedFiles;
 #endif
 
-////////////////////////////////////////////////////////////////////////////////
 /// @brief lock for protected access to vector OpenedFiles
-////////////////////////////////////////////////////////////////////////////////
-
 static basics::ReadWriteLock OpenedFilesLock;
 
+/// @brief struct to remove all opened lockfiles on shutdown
+struct LockfileRemover {
+  LockfileRemover() {}
+
+  ~LockfileRemover() {
+    WRITE_LOCKER(locker, OpenedFilesLock);
+
+    for (auto const& it : OpenedFiles) {
+#ifdef TRI_HAVE_WIN32_FILE_LOCKING
+      HANDLE fd = it.second;
+      CloseHandle(fd);
+#else
+      int fd = it.second;
+      TRI_TRACKED_CLOSE_FILE(fd);
+#endif
+
+      TRI_UnlinkFile(it.first.c_str());
+    }
+
+    OpenedFiles.clear();
+  }
+};
+
+/// @brief this instance will remove all lockfiles in its dtor
+static LockfileRemover remover;
+}
+
+/// @brief read buffer size (used for bulk file reading)
+#define READBUFFER_SIZE 8192
+
+/// @brief a static buffer of zeros, used to initialize files
+static char NullBuffer[4096];
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief whether or not the character is a directory separator
 ////////////////////////////////////////////////////////////////////////////////
@@ -111,41 +122,6 @@ static void NormalizePath(std::string& path) {
   }
 }
 
-////////////////////////////////////////////////////////////////////////////////
-/// @brief callback function for removing all locked files by a process
-////////////////////////////////////////////////////////////////////////////////
-
-static void RemoveAllLockedFiles(void) {
-  WRITE_LOCKER(locker, OpenedFilesLock);
-
-  for (auto const& it : OpenedFiles) {
-#ifdef TRI_HAVE_WIN32_FILE_LOCKING
-    HANDLE fd = it.second;
-    CloseHandle(fd);
-#else
-    int fd = it.second;
-    TRI_TRACKED_CLOSE_FILE(fd);
-#endif
-
-    TRI_UnlinkFile(it.first.c_str());
-  }
-
-  OpenedFiles.clear();
-}
-
-////////////////////////////////////////////////////////////////////////////////
-/// @brief initializes some structures which are needed by the file functions
-////////////////////////////////////////////////////////////////////////////////
-
-static void InitializeLockFiles(void) {
-  if (Initialized) {
-    return;
-  }
-
-  Initialized = true;
-  atexit(&RemoveAllLockedFiles);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief lists the directory tree
 ////////////////////////////////////////////////////////////////////////////////
@@ -981,10 +957,8 @@ int TRI_CreateLockFile(char const* filename) {
     }
   }
   
-  InitializeLockFiles();
-
   HANDLE fd = CreateFile(filename, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS,
-                  FILE_ATTRIBUTE_NORMAL, NULL);
+                         FILE_ATTRIBUTE_NORMAL, NULL);
 
   if (fd == INVALID_HANDLE_VALUE) {
     TRI_SYSTEM_ERROR();
@@ -1047,8 +1021,6 @@ int TRI_CreateLockFile(char const* filename) {
     }
   }
   
-  InitializeLockFiles();
-
   int fd = TRI_TRACKED_CREATE_FILE(filename, O_CREAT | O_EXCL | O_RDWR | TRI_O_CLOEXEC,
                       S_IRUSR | S_IWUSR);
 
@@ -1227,8 +1199,6 @@ int TRI_VerifyLockFile(char const* filename) {
 #ifdef TRI_HAVE_WIN32_FILE_LOCKING
 
 int TRI_DestroyLockFile(char const* filename) {
-  InitializeLockFiles();
-  
   WRITE_LOCKER(locker, OpenedFilesLock);
   for (size_t i = 0; i < OpenedFiles.size(); ++i) {
     if (OpenedFiles[i].first == filename) {
@@ -1247,8 +1217,6 @@ int TRI_DestroyLockFile(char const* filename) {
 #else
 
 int TRI_DestroyLockFile(char const* filename) {
-  InitializeLockFiles();
-  
   WRITE_LOCKER(locker, OpenedFilesLock);
   for (size_t i = 0; i < OpenedFiles.size(); ++i) {
     if (OpenedFiles[i].first == filename) {