Skip to content

Commit

Permalink
fix(sysutil): improve the implementation of Sysutil::put_in_backgroun…
Browse files Browse the repository at this point in the history
…d on macOS (#4640)

Replaces the system() call with posix_spawn() on macOS to preserve
environment variables. This is essential for Debug builds using
DYLD_IMAGE_SUFFIX=_debug to prevent Qt from loading mixed
configurations.

## Tests

Run the iv application in background mode on macOS without the -F flag,
using a Debug configuration. Previously, the system() call did not
inherit the parent environment variables, causing a mix of Debug and
Release versions of Qt to be loaded.

---------

Signed-off-by: Mikael Sundell <[email protected]>
  • Loading branch information
mikaelsundell authored Feb 23, 2025
1 parent 122e6ae commit d87412f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 34 deletions.
14 changes: 11 additions & 3 deletions src/include/OpenImageIO/sysutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,19 @@ getenv(string_view name, string_view defaultval = {});
OIIO_UTIL_API void
usleep(unsigned long useconds);

/// Try to put the process into the background so it doesn't continue to
/// tie up any shell that it was launched from. The arguments are the
/// argc/argv that describe the program and its command line arguments.
/// Puts the process into the background, detaching it from the shell
/// to prevent it from occupying the terminal or blocking the shell
/// it was launched from. This allows the process to continue running
/// independently in the background.
/// Return true if successful, false if it was unable to do so.
OIIO_UTIL_API bool
put_in_background();

/// Obsolete version. The argc, argv parameters are not used. We suggest
/// switching to the version of `put_in_background()` that takes no
/// arguments.
/// DEPRECATED(3.0) old API.
OIIO_UTIL_API bool
put_in_background(int argc, char* argv[]);

/// Number of virtual cores available on this platform (including
Expand Down
2 changes: 1 addition & 1 deletion src/iv/ivmain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ main(int argc, char* argv[])
ArgParse ap = getargs(argc, argv);

if (!ap["foreground_mode"].get<int>())
Sysutil::put_in_background(argc, argv);
Sysutil::put_in_background();

// LG
// Q_INIT_RESOURCE(iv);
Expand Down
56 changes: 26 additions & 30 deletions src/libutil/sysutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@

#ifdef __APPLE__
# include <TargetConditionals.h>
# include <crt_externs.h>
# include <mach-o/dyld.h>
# include <mach/mach_init.h>
# include <mach/task.h>
# include <spawn.h>
# include <sys/ioctl.h>
# include <sys/sysctl.h>
# include <unistd.h>
Expand Down Expand Up @@ -523,46 +525,33 @@ Term::ansi_bgcolor(int r, int g, int b)
return ret;
}



bool
#ifdef _WIN32
Sysutil::put_in_background(int, char*[])
#else
Sysutil::put_in_background(int argc, char* argv[])
#endif
Sysutil::put_in_background()
{
// You would think that this would be sufficient:
// pid_t pid = fork ();
// if (pid < 0) // Some kind of error, we were unable to background
// return false;
// if (pid == 0)
// return true; // This is the child process, so continue with life
// // Otherwise, this is the parent process, so terminate
// exit (0);
// But it's not. On OS X, it's not safe to fork() if your app is linked
// against certain libraries or frameworks. So the only thing that I
// think is safe is to exec a new process.
// Another solution is this:
// daemon (1, 1);
// But it suffers from the same problem on OS X, and seems to just be
// a wrapper for fork.

#if defined(__linux__) || defined(__GLIBC__)
// Simplest case:
// daemon returns 0 if successful, thus return true if successful
return daemon(1, 1) == 0;

#elif defined(__APPLE__) && TARGET_OS_OSX
std::string newcmd = std::string(argv[0]) + " -F";
for (int i = 1; i < argc; ++i) {
newcmd += " \"";
newcmd += argv[i];
newcmd += "\"";
// On macOS, check if the parent process is launchd (PID 1).
// If getppid() == 1, the current process is already detached from the shell
// and running in the background. Returning early prevents spawning
// another background process, avoiding infinite recursion.
if (getppid() == 1) {
return true;
}
newcmd += " &";
if (system(newcmd.c_str()) != -1)
pid_t pid;
posix_spawnattr_t attr;
posix_spawnattr_init(&attr);
posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSID);
char** argv = *_NSGetArgv();
char** environ = *_NSGetEnviron();
int status = posix_spawn(&pid, argv[0], nullptr, &attr, argv, environ);
posix_spawnattr_destroy(&attr);
if (status == 0)
exit(0);

return true;

#elif defined(_WIN32)
Expand All @@ -575,6 +564,13 @@ Sysutil::put_in_background(int argc, char* argv[])
}


bool
Sysutil::put_in_background(int argc, char* argv[])
{
return put_in_background();
}



unsigned int
Sysutil::hardware_concurrency()
Expand Down

0 comments on commit d87412f

Please sign in to comment.