Make fs::extractZipFile thread-safe

diff --git a/src/filesys.cpp b/src/filesys.cpp
index a07370c0eb54c34f491140e8abea616014092a82..0972acbf95eded92c8c693ae4b0062e1976511b9 100644 (file)
--- a/src/filesys.cpp
+++ b/src/filesys.cpp
@@ -28,11 +28,18 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "log.h"
 #include "config.h"
 #include "porting.h"
+#ifndef SERVER
+#include "irr_ptr.h"
+#endif
 
 namespace fs
 {
 
-#ifdef _WIN32 // WINDOWS
+#ifdef _WIN32
+
+/***********
+ * Windows *
+ ***********/
 
 #define _WIN32_WINNT 0x0501
 #include <windows.h>
@@ -201,7 +208,11 @@ std::string CreateTempFile()
        return path;
 }
 
-#else // POSIX
+#else
+
+/*********
+ * POSIX *
+ *********/
 
 #include <sys/types.h>
 #include <dirent.h>
@@ -392,6 +403,10 @@ std::string CreateTempFile()
 
 #endif
 
+/****************************
+ * portable implementations *
+ ****************************/
+
 void GetRecursiveDirs(std::vector<std::string> &dirs, const std::string &dir)
 {
        static const std::set<char> chars_to_ignore = { '_', '.' };
@@ -753,69 +768,66 @@ bool safeWriteToFile(const std::string &path, const std::string &content)
        return true;
 }
 
+#ifndef SERVER
 bool extractZipFile(io::IFileSystem *fs, const char *filename, const std::string &destination)
 {
-       if (!fs->addFileArchive(filename, false, false, io::EFAT_ZIP)) {
+       // Be careful here not to touch the global file hierarchy in Irrlicht
+       // since this function needs to be thread-safe!
+
+       io::IArchiveLoader *zip_loader = nullptr;
+       for (u32 i = 0; i < fs->getArchiveLoaderCount(); i++) {
+               if (fs->getArchiveLoader(i)->isALoadableFileFormat(io::EFAT_ZIP)) {
+                       zip_loader = fs->getArchiveLoader(i);
+                       break;
+               }
+       }
+       if (!zip_loader) {
+               warningstream << "fs::extractZipFile(): Irrlicht said it doesn't support ZIPs." << std::endl;
                return false;
        }
 
-       sanity_check(fs->getFileArchiveCount() > 0);
-
-       /**********************************************************************/
-       /* WARNING this is not threadsafe!!                                   */
-       /**********************************************************************/
-       io::IFileArchive* opened_zip = fs->getFileArchive(fs->getFileArchiveCount() - 1);
-
+       irr_ptr<io::IFileArchive> opened_zip(zip_loader->createArchive(filename, false, false));
        const io::IFileList* files_in_zip = opened_zip->getFileList();
 
-       unsigned int number_of_files = files_in_zip->getFileCount();
-
-       for (unsigned int i=0; i < number_of_files; i++) {
-               std::string fullpath = destination;
-               fullpath += DIR_DELIM;
+       for (u32 i = 0; i < files_in_zip->getFileCount(); i++) {
+               std::string fullpath = destination + DIR_DELIM;
                fullpath += files_in_zip->getFullFileName(i).c_str();
                std::string fullpath_dir = fs::RemoveLastPathComponent(fullpath);
 
-               if (!files_in_zip->isDirectory(i)) {
-                       if (!fs::PathExists(fullpath_dir) && !fs::CreateAllDirs(fullpath_dir)) {
-                               fs->removeFileArchive(fs->getFileArchiveCount()-1);
-                               return false;
-                       }
-
-                       io::IReadFile* toread = opened_zip->createAndOpenFile(i);
+               if (files_in_zip->isDirectory(i))
+                       continue; // ignore, we create dirs as necessary
 
-                       FILE *targetfile = fopen(fullpath.c_str(),"wb");
+               if (!fs::PathExists(fullpath_dir) && !fs::CreateAllDirs(fullpath_dir))
+                       return false;
 
-                       if (targetfile == NULL) {
-                               fs->removeFileArchive(fs->getFileArchiveCount()-1);
-                               return false;
-                       }
+               irr_ptr<io::IReadFile> toread(opened_zip->createAndOpenFile(i));
 
-                       char read_buffer[1024];
-                       long total_read = 0;
+               std::ofstream os(fullpath.c_str(), std::ios::binary);
+               if (!os.good())
+                       return false;
 
-                       while (total_read < toread->getSize()) {
+               char buffer[4096];
+               long total_read = 0;
 
-                               unsigned int bytes_read =
-                                               toread->read(read_buffer,sizeof(read_buffer));
-                               if ((bytes_read == 0 ) ||
-                                       (fwrite(read_buffer, 1, bytes_read, targetfile) != bytes_read))
-                               {
-                                       fclose(targetfile);
-                                       fs->removeFileArchive(fs->getFileArchiveCount() - 1);
-                                       return false;
-                               }
-                               total_read += bytes_read;
+               while (total_read < toread->getSize()) {
+                       long bytes_read = toread->read(buffer, sizeof(buffer));
+                       bool error = true;
+                       if (bytes_read != 0) {
+                               os.write(buffer, bytes_read);
+                               error = os.fail();
                        }
-
-                       fclose(targetfile);
+                       if (error) {
+                               os.close();
+                               remove(fullpath.c_str());
+                               return false;
+                       }
+                       total_read += bytes_read;
                }
-
        }
 
-       fs->removeFileArchive(fs->getFileArchiveCount() - 1);
        return true;
 }
+#endif
 
 bool ReadFile(const std::string &path, std::string &out)
 {
@@ -829,7 +841,7 @@ bool ReadFile(const std::string &path, std::string &out)
        is.seekg(0);
        is.read(&out[0], size);
 
-       return true;
+       return !is.fail();
 }
 
 bool Rename(const std::string &from, const std::string &to)

diff --git a/src/filesys.h b/src/filesys.h
index f72cb0ba2822a8e88e5ded2ec5d666b2c11acd27..233e56bba4cb2823f63bcc11d829652ec3197ee9 100644 (file)
--- a/src/filesys.h
+++ b/src/filesys.h
@@ -24,12 +24,12 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <vector>
 #include "exceptions.h"
 
-#ifdef _WIN32 // WINDOWS
+#ifdef _WIN32
 #define DIR_DELIM "\\"
 #define DIR_DELIM_CHAR '\\'
 #define FILESYS_CASE_INSENSITIVE true
 #define PATH_DELIM ";"
-#else // POSIX
+#else
 #define DIR_DELIM "/"
 #define DIR_DELIM_CHAR '/'
 #define FILESYS_CASE_INSENSITIVE false
@@ -133,7 +133,9 @@ const char *GetFilenameFromPath(const char *path);
 
 bool safeWriteToFile(const std::string &path, const std::string &content);
 
+#ifndef SERVER
 bool extractZipFile(irr::io::IFileSystem *fs, const char *filename, const std::string &destination);
+#endif
 
 bool ReadFile(const std::string &path, std::string &out);
 

diff --git a/src/script/lua_api/l_mainmenu.cpp b/src/script/lua_api/l_mainmenu.cpp
index 57fddc0be32d96f930128199dcf09bb52cc1825b..4cfbaec711fbc378c30f9b5232df73b02acf66a4 100644 (file)
--- a/src/script/lua_api/l_mainmenu.cpp
+++ b/src/script/lua_api/l_mainmenu.cpp
@@ -644,9 +644,9 @@ int ModApiMainMenu::l_extract_zip(lua_State *L)
        std::string absolute_destination = fs::RemoveRelativePathComponents(destination);
 
        if (ModApiMainMenu::mayModifyPath(absolute_destination)) {
-               auto rendering_engine = getGuiEngine(L)->m_rendering_engine;
-               fs::CreateAllDirs(absolute_destination);
-               lua_pushboolean(L, fs::extractZipFile(rendering_engine->get_filesystem(), zipfile, destination));
+               auto fs = RenderingEngine::get_raw_device()->getFileSystem();
+               bool ok = fs::extractZipFile(fs, zipfile, destination);
+               lua_pushboolean(L, ok);
                return 1;
        }
 
@@ -916,7 +916,7 @@ void ModApiMainMenu::InitializeAsync(lua_State *L, int top)
        API_FCT(delete_dir);
        API_FCT(copy_dir);
        API_FCT(is_dir);
-       //API_FCT(extract_zip); //TODO remove dependency to GuiEngine
+       API_FCT(extract_zip);
        API_FCT(may_modify_path);
        API_FCT(download_file);
        API_FCT(get_min_supp_proto);