Ghost: Make special user directory query thread safe

Similar to e0ff7731e0.

Noticed a data race when working on blender/blender!130543, which calls
this function from a thread. Make this thread safe by avoiding returning
of static memory, instead use an optional `std::string`.

Pull Request: https://projects.blender.org/blender/blender/pulls/141083
This commit is contained in:
Julian Eisel
2025-06-27 15:16:35 +02:00
committed by Julian Eisel
parent 52caedb19e
commit 4b2d60a2c3
10 changed files with 71 additions and 37 deletions

View File

@@ -8,6 +8,9 @@
#pragma once #pragma once
#include <optional>
#include <string>
#include "GHOST_Types.h" #include "GHOST_Types.h"
class GHOST_ISystemPaths { class GHOST_ISystemPaths {
@@ -60,9 +63,9 @@ class GHOST_ISystemPaths {
/** /**
* Determine a special ("well known") and easy to reach user directory. * Determine a special ("well known") and easy to reach user directory.
* \return Unsigned char string pointing to user directory (eg `~/Documents/`). * \return If successfull, a string containing the user directory path (eg `~/Documents/`).
*/ */
virtual const char *getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const = 0; virtual std::optional<std::string> getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const = 0;
/** /**
* Determine the directory of the current binary * Determine the directory of the current binary

View File

@@ -8,6 +8,9 @@
#pragma once #pragma once
#include <optional>
#include <string>
#include "GHOST_Types.h" #include "GHOST_Types.h"
GHOST_DECLARE_HANDLE(GHOST_SystemPathsHandle); GHOST_DECLARE_HANDLE(GHOST_SystemPathsHandle);
@@ -46,9 +49,9 @@ extern const char *GHOST_getUserDir(int version, const char *versionstr);
/** /**
* Determine a special ("well known") and easy to reach user directory. * Determine a special ("well known") and easy to reach user directory.
* \return Unsigned char string pointing to user directory (eg `~/Documents/`). * \return If successfull, a string containing the user directory path (eg `~/Documents/`).
*/ */
extern const char *GHOST_getUserSpecialDir(GHOST_TUserSpecialDirTypes type); extern std::optional<std::string> GHOST_getUserSpecialDir(GHOST_TUserSpecialDirTypes type);
/** /**
* Determine the directory in which the binary file is found. * Determine the directory in which the binary file is found.

View File

@@ -34,7 +34,7 @@ const char *GHOST_getUserDir(int version, const char *versionstr)
return systemPaths ? systemPaths->getUserDir(version, versionstr) : nullptr; return systemPaths ? systemPaths->getUserDir(version, versionstr) : nullptr;
} }
const char *GHOST_getUserSpecialDir(GHOST_TUserSpecialDirTypes type) std::optional<std::string> GHOST_getUserSpecialDir(GHOST_TUserSpecialDirTypes type)
{ {
const GHOST_ISystemPaths *systemPaths = GHOST_ISystemPaths::get(); const GHOST_ISystemPaths *systemPaths = GHOST_ISystemPaths::get();
/* Shouldn't be `nullptr`. */ /* Shouldn't be `nullptr`. */

View File

@@ -12,6 +12,9 @@
# error Apple OSX only! # error Apple OSX only!
#endif // __APPLE__ #endif // __APPLE__
#include <optional>
#include <string>
#include "GHOST_SystemPaths.hh" #include "GHOST_SystemPaths.hh"
class GHOST_SystemPathsCocoa : public GHOST_SystemPaths { class GHOST_SystemPathsCocoa : public GHOST_SystemPaths {
@@ -42,9 +45,9 @@ class GHOST_SystemPathsCocoa : public GHOST_SystemPaths {
/** /**
* Determine a special ("well known") and easy to reach user directory. * Determine a special ("well known") and easy to reach user directory.
* \return Unsigned char string pointing to user directory (eg `~/Documents/`). * \return If successfull, a string containing the user directory path (eg `~/Documents/`).
*/ */
const char *getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const override; std::optional<std::string> getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const override;
/** /**
* Determine the directory of the current binary. * Determine the directory of the current binary.

View File

@@ -5,6 +5,9 @@
#import <AppKit/NSDocumentController.h> #import <AppKit/NSDocumentController.h>
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#include <optional>
#include <string>
#include "GHOST_Debug.hh" #include "GHOST_Debug.hh"
#include "GHOST_SystemPathsCocoa.hh" #include "GHOST_SystemPathsCocoa.hh"
@@ -46,9 +49,10 @@ const char *GHOST_SystemPathsCocoa::getUserDir(int /* version */, const char *ve
return GetApplicationSupportDir(versionstr, NSUserDomainMask, tempPath, sizeof(tempPath)); return GetApplicationSupportDir(versionstr, NSUserDomainMask, tempPath, sizeof(tempPath));
} }
const char *GHOST_SystemPathsCocoa::getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const std::optional<std::string> GHOST_SystemPathsCocoa::getUserSpecialDir(
GHOST_TUserSpecialDirTypes type) const
{ {
static char tempPath[512] = ""; char tempPath[512] = "";
@autoreleasepool { @autoreleasepool {
NSSearchPathDirectory ns_directory; NSSearchPathDirectory ns_directory;
@@ -78,12 +82,12 @@ const char *GHOST_SystemPathsCocoa::getUserSpecialDir(GHOST_TUserSpecialDirTypes
GHOST_ASSERT( GHOST_ASSERT(
false, false,
"GHOST_SystemPathsCocoa::getUserSpecialDir(): Invalid enum value for type parameter"); "GHOST_SystemPathsCocoa::getUserSpecialDir(): Invalid enum value for type parameter");
return nullptr; return std::nullopt;
} }
NSArray *paths = NSSearchPathForDirectoriesInDomains(ns_directory, NSUserDomainMask, YES); NSArray *paths = NSSearchPathForDirectoriesInDomains(ns_directory, NSUserDomainMask, YES);
if (paths.count == 0) { if (paths.count == 0) {
return nullptr; return std::nullopt;
} }
NSString *basePath = [paths objectAtIndex:0]; NSString *basePath = [paths objectAtIndex:0];
@@ -94,6 +98,9 @@ const char *GHOST_SystemPathsCocoa::getUserSpecialDir(GHOST_TUserSpecialDirTypes
memcpy(tempPath, basePath_cstr, basePath_len); memcpy(tempPath, basePath_cstr, basePath_len);
tempPath[basePath_len] = '\0'; tempPath[basePath_len] = '\0';
} }
if (!tempPath[0]) {
return std::nullopt;
}
return tempPath; return tempPath;
} }

View File

@@ -6,6 +6,7 @@
* \ingroup GHOST * \ingroup GHOST
*/ */
#include <optional>
#include <sstream> #include <sstream>
#include "GHOST_SystemPathsUnix.hh" #include "GHOST_SystemPathsUnix.hh"
@@ -100,10 +101,10 @@ const char *GHOST_SystemPathsUnix::getUserDir(int version, const char *versionst
return user_path.c_str(); return user_path.c_str();
} }
const char *GHOST_SystemPathsUnix::getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const std::optional<std::string> GHOST_SystemPathsUnix::getUserSpecialDir(
GHOST_TUserSpecialDirTypes type) const
{ {
const char *type_str; const char *type_str;
static string path;
switch (type) { switch (type) {
case GHOST_kUserSpecialDirDesktop: case GHOST_kUserSpecialDirDesktop:
@@ -133,16 +134,15 @@ const char *GHOST_SystemPathsUnix::getUserSpecialDir(GHOST_TUserSpecialDirTypes
/* If `XDG_CACHE_HOME` is not set, then `$HOME/.cache is used`. */ /* If `XDG_CACHE_HOME` is not set, then `$HOME/.cache is used`. */
const char *home_dir = home_dir_get(); const char *home_dir = home_dir_get();
if (home_dir == nullptr) { if (home_dir == nullptr) {
return nullptr; return std::nullopt;
} }
path = string(home_dir) + "/.cache"; return string(home_dir) + "/.cache";
return path.c_str();
} }
default: default:
GHOST_ASSERT( GHOST_ASSERT(
false, false,
"GHOST_SystemPathsUnix::getUserSpecialDir(): Invalid enum value for type parameter"); "GHOST_SystemPathsUnix::getUserSpecialDir(): Invalid enum value for type parameter");
return nullptr; return std::nullopt;
} }
/* Pipe `stderr` to `/dev/null` to avoid error prints. We will fail gracefully still. */ /* Pipe `stderr` to `/dev/null` to avoid error prints. We will fail gracefully still. */
@@ -150,7 +150,7 @@ const char *GHOST_SystemPathsUnix::getUserSpecialDir(GHOST_TUserSpecialDirTypes
FILE *fstream = popen(command.c_str(), "r"); FILE *fstream = popen(command.c_str(), "r");
if (fstream == nullptr) { if (fstream == nullptr) {
return nullptr; return std::nullopt;
} }
std::stringstream path_stream; std::stringstream path_stream;
while (!feof(fstream)) { while (!feof(fstream)) {
@@ -163,11 +163,11 @@ const char *GHOST_SystemPathsUnix::getUserSpecialDir(GHOST_TUserSpecialDirTypes
} }
if (pclose(fstream) == -1) { if (pclose(fstream) == -1) {
perror("GHOST_SystemPathsUnix::getUserSpecialDir failed at pclose()"); perror("GHOST_SystemPathsUnix::getUserSpecialDir failed at pclose()");
return nullptr; return std::nullopt;
} }
path = path_stream.str(); std::string path = path_stream.str();
return path[0] ? path.c_str() : nullptr; return path[0] ? std::optional(path) : std::nullopt;
} }
const char *GHOST_SystemPathsUnix::getBinaryDir() const const char *GHOST_SystemPathsUnix::getBinaryDir() const

View File

@@ -8,6 +8,9 @@
#pragma once #pragma once
#include <optional>
#include <string>
#include "../GHOST_Types.h" #include "../GHOST_Types.h"
#include "GHOST_SystemPaths.hh" #include "GHOST_SystemPaths.hh"
@@ -40,9 +43,9 @@ class GHOST_SystemPathsUnix : public GHOST_SystemPaths {
/** /**
* Determine a special ("well known") and easy to reach user directory. * Determine a special ("well known") and easy to reach user directory.
* \return Unsigned char string pointing to user directory (eg `~/Documents/`). * \return If successfull, a string containing the user directory path (eg `~/Documents/`).
*/ */
const char *getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const override; std::optional<std::string> getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const override;
/** /**
* Determine the directory of the current binary. * Determine the directory of the current binary.

View File

@@ -62,7 +62,8 @@ const char *GHOST_SystemPathsWin32::getUserDir(int, const char *versionstr) cons
return user_dir; return user_dir;
} }
const char *GHOST_SystemPathsWin32::getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const std::optional<std::string> GHOST_SystemPathsWin32::getUserSpecialDir(
GHOST_TUserSpecialDirTypes type) const
{ {
const char *special_dir = nullptr; const char *special_dir = nullptr;
@@ -93,7 +94,7 @@ const char *GHOST_SystemPathsWin32::getUserSpecialDir(GHOST_TUserSpecialDirTypes
GHOST_ASSERT( GHOST_ASSERT(
false, false,
"GHOST_SystemPathsWin32::getUserSpecialDir(): Invalid enum value for type parameter"); "GHOST_SystemPathsWin32::getUserSpecialDir(): Invalid enum value for type parameter");
return nullptr; return std::nullopt;
} }
static char knownpath[MAX_PATH * 3] = {0}; static char knownpath[MAX_PATH * 3] = {0};
@@ -105,6 +106,10 @@ const char *GHOST_SystemPathsWin32::getUserSpecialDir(GHOST_TUserSpecialDirTypes
special_dir = knownpath; special_dir = knownpath;
} }
if ((special_dir == nullptr) || (special_dir[0] == '\0')) {
return std::nullopt;
}
CoTaskMemFree(knownpath_16); CoTaskMemFree(knownpath_16);
return special_dir; return special_dir;
} }

View File

@@ -8,6 +8,9 @@
#pragma once #pragma once
#include <optional>
#include <string>
#ifndef WIN32 #ifndef WIN32
# error WIN32 only! # error WIN32 only!
#endif // WIN32 #endif // WIN32
@@ -49,9 +52,9 @@ class GHOST_SystemPathsWin32 : public GHOST_SystemPaths {
/** /**
* Determine a special ("well known") and easy to reach user directory. * Determine a special ("well known") and easy to reach user directory.
* \return Unsigned char string pointing to user directory (eg `~/Documents/`). * \return If successfull, a string containing the user directory path (eg `~/Documents/`).
*/ */
const char *getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const override; std::optional<std::string> getUserSpecialDir(GHOST_TUserSpecialDirTypes type) const override;
/** /**
* Determine the directory of the current binary. * Determine the directory of the current binary.

View File

@@ -168,11 +168,12 @@ bool BKE_appdir_folder_documents(char *dir)
{ {
dir[0] = '\0'; dir[0] = '\0';
const char *documents_path = GHOST_getUserSpecialDir(GHOST_kUserSpecialDirDocuments); const std::optional<std::string> documents_path = GHOST_getUserSpecialDir(
GHOST_kUserSpecialDirDocuments);
/* Usual case: Ghost gave us the documents path. We're done here. */ /* Usual case: Ghost gave us the documents path. We're done here. */
if (documents_path && BLI_is_dir(documents_path)) { if (documents_path && BLI_is_dir(documents_path->c_str())) {
BLI_strncpy(dir, documents_path, FILE_MAXDIR); BLI_strncpy(dir, documents_path->c_str(), FILE_MAXDIR);
return true; return true;
} }
@@ -198,21 +199,27 @@ bool BKE_appdir_folder_caches(char *path, const size_t path_maxncpy)
{ {
path[0] = '\0'; path[0] = '\0';
const char *caches_root_path = GHOST_getUserSpecialDir(GHOST_kUserSpecialDirCaches); std::optional<std::string> caches_root_path = GHOST_getUserSpecialDir(
if (caches_root_path == nullptr || !BLI_is_dir(caches_root_path)) { GHOST_kUserSpecialDirCaches);
if (!caches_root_path || !BLI_is_dir(caches_root_path->c_str())) {
caches_root_path = BKE_tempdir_base(); caches_root_path = BKE_tempdir_base();
} }
if (caches_root_path == nullptr || !BLI_is_dir(caches_root_path)) { if (!caches_root_path || !BLI_is_dir(caches_root_path->c_str())) {
return false; return false;
} }
#ifdef WIN32 #ifdef WIN32
BLI_path_join( BLI_path_join(path,
path, path_maxncpy, caches_root_path, "Blender Foundation", "Blender", "Cache", SEP_STR); path_maxncpy,
caches_root_path->c_str(),
"Blender Foundation",
"Blender",
"Cache",
SEP_STR);
#elif defined(__APPLE__) #elif defined(__APPLE__)
BLI_path_join(path, path_maxncpy, caches_root_path, "Blender", SEP_STR); BLI_path_join(path, path_maxncpy, caches_root_path->c_str(), "Blender", SEP_STR);
#else /* __linux__ */ #else /* __linux__ */
BLI_path_join(path, path_maxncpy, caches_root_path, "blender", SEP_STR); BLI_path_join(path, path_maxncpy, caches_root_path->c_str(), "blender", SEP_STR);
#endif #endif
return true; return true;