mirror of
https://github.com/NixOS/nix
synced 2025-06-24 22:11:15 +02:00
libutil: thread-pool: ensure threads finished on error
This fixes segfaults with nix copy when there was an error processing addMultipleToStore. Running with ASAN/TSAN pointed at an use-after-free with threads from the pool accessing the graph declared in processGraph after the function was exiting and destructing the variables. It turns out that if there is an error before pool.process() is called, for example while we are still enqueuing tasks, then pool.process() isn't called and threads are still left to run. By creating the pool last we ensure that it is stopped first before running other destructors even if an exception happens early. [ lix porting note: nix does not name threads so the patch has been adapted to not pass thread name ] Link: https://git.lix.systems/lix-project/lix/issues/618 Link: https://gerrit.lix.systems/c/lix/+/2355
This commit is contained in:
parent
a44ae8b5a9
commit
afac093b34
2 changed files with 6 additions and 7 deletions
|
@ -246,9 +246,7 @@ void Store::addMultipleToStore(
|
|||
act.progress(nrDone, pathsToCopy.size(), nrRunning, nrFailed);
|
||||
};
|
||||
|
||||
ThreadPool pool;
|
||||
|
||||
processGraph<StorePath>(pool,
|
||||
processGraph<StorePath>(
|
||||
storePathsToAdd,
|
||||
|
||||
[&](const StorePath & path) {
|
||||
|
@ -1028,12 +1026,10 @@ std::map<StorePath, StorePath> copyPaths(
|
|||
}
|
||||
auto pathsMap = copyPaths(srcStore, dstStore, storePaths, repair, checkSigs, substitute);
|
||||
|
||||
ThreadPool pool;
|
||||
|
||||
try {
|
||||
// Copy the realisation closure
|
||||
processGraph<Realisation>(
|
||||
pool, Realisation::closure(srcStore, toplevelRealisations),
|
||||
Realisation::closure(srcStore, toplevelRealisations),
|
||||
[&](const Realisation & current) -> std::set<Realisation> {
|
||||
std::set<Realisation> children;
|
||||
for (const auto & [drvOutput, _] : current.dependentRealisations) {
|
||||
|
|
|
@ -83,7 +83,6 @@ private:
|
|||
*/
|
||||
template<typename T>
|
||||
void processGraph(
|
||||
ThreadPool & pool,
|
||||
const std::set<T> & nodes,
|
||||
std::function<std::set<T>(const T &)> getEdges,
|
||||
std::function<void(const T &)> processNode)
|
||||
|
@ -97,6 +96,10 @@ void processGraph(
|
|||
|
||||
std::function<void(const T &)> worker;
|
||||
|
||||
/* Create pool last to ensure threads are stopped before other destructors
|
||||
* run */
|
||||
ThreadPool pool;
|
||||
|
||||
worker = [&](const T & node) {
|
||||
|
||||
{
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue