Then jobserver is initialized from pipe, it sets cloexec, but doesn't delete environment variable which points to file descriptors. By default (unless Client::configure is used), child process will be instructed (if it uses jobserver) to access file descriptors that was closed and maybe reopened to point to other files.
This seems like a violation of IO-safety.
Either cloexec shouldn't be set or the environment variables should also be removed and only be added back via configure.
This pr is aimed to implement first option.
Concerns:
current behavior is how gnu make works,
it may be breaking change.
Reasons to choose first option:
in a typical case inheritance is what we want - inheritance by default will reduce amount of places in code where jobserver code is called,
fifo-style (named pipes) jobserver is inherited by default:
it's weird to have different behaviors depending on implementation details,
modern posix gnu make uses this fifo style by default, so it may be more common in the future,
second option requires changing (more) global things (cloexec, environment variable).
Concerns:
with inheritance by default, how to work with multiply jobservers at once, if it would be necessary?
with inheritance by default, Client::new in current pr's code changes global state - env var.
Notes:
pr is a draft, in particular, documentation wasn't updated,
windows implementation creates semaphore without SECURITY_ATTRIBUTES::bInheritHandle and does nothing in Client::configure. According to learn.microsoft.com, the handle cannot be inherited by child processes.
From #64:
Then jobserver is initialized from pipe, it sets cloexec, but doesn't delete environment variable which points to file descriptors. By default (unless
Client::configureis used), child process will be instructed (if it uses jobserver) to access file descriptors that was closed and maybe reopened to point to other files.This seems like a violation of IO-safety.
Either cloexec shouldn't be set or the environment variables should also be removed and only be added back via
configure.This pr is aimed to implement first option.
Concerns:
Reasons to choose first option:
Concerns:
Client::newin current pr's code changes global state - env var.Notes:
SECURITY_ATTRIBUTES::bInheritHandleand does nothing inClient::configure. According to learn.microsoft.com, the handle cannot be inherited by child processes.