* In addPermRoot, check that the root that we just registered can be
found by the garbage collector. This addresses NIX-71 and is a particular concern in multi-user stores.
This commit is contained in:
		
							parent
							
								
									74033a844f
								
							
						
					
					
						commit
						d27a73b1a9
					
				
					 2 changed files with 42 additions and 18 deletions
				
			
		|  | @ -47,7 +47,7 @@ void printGCWarning() | ||||||
| { | { | ||||||
|     static bool haveWarned = false; |     static bool haveWarned = false; | ||||||
|     warnOnce(haveWarned,  |     warnOnce(haveWarned,  | ||||||
|         "warning: you did not specify `--add-root'; " |         "you did not specify `--add-root'; " | ||||||
|         "the result might be removed by the garbage collector"); |         "the result might be removed by the garbage collector"); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -91,6 +91,9 @@ void LocalStore::addIndirectRoot(const Path & path) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  | static void findRoots(PathSet & roots, bool ignoreUnreadable); | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
| Path addPermRoot(const Path & _storePath, const Path & _gcRoot, | Path addPermRoot(const Path & _storePath, const Path & _gcRoot, | ||||||
|     bool indirect, bool allowOutsideRootsDir) |     bool indirect, bool allowOutsideRootsDir) | ||||||
| { | { | ||||||
|  | @ -115,6 +118,18 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot, | ||||||
|         } |         } | ||||||
|              |              | ||||||
|         createSymlink(gcRoot, storePath, false); |         createSymlink(gcRoot, storePath, false); | ||||||
|  | 
 | ||||||
|  |         /* Check that the root can be found by the garbage collector. */ | ||||||
|  |         PathSet roots; | ||||||
|  |         findRoots(roots, true); | ||||||
|  |         if (roots.find(storePath) == roots.end()) { | ||||||
|  |             static bool haveWarned = false; | ||||||
|  |             warnOnce(haveWarned,  | ||||||
|  |                 format( | ||||||
|  |                     "the path `%1%' is not reachable from the roots directory of the garbage collector; " | ||||||
|  |                     "therefore, `%2%' might be removed by the garbage collector") | ||||||
|  |                 % gcRoot % storePath); | ||||||
|  |         } | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     /* Grab the global GC root, causing us to block while a GC is in
 |     /* Grab the global GC root, causing us to block while a GC is in
 | ||||||
|  | @ -292,7 +307,7 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds) | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| static void findRoots(const Path & path, bool recurseSymlinks, | static void findRoots(const Path & path, bool recurseSymlinks, | ||||||
|     PathSet & roots) |     bool ignoreUnreadable, PathSet & roots) | ||||||
| { | { | ||||||
|     struct stat st; |     struct stat st; | ||||||
|     if (lstat(path.c_str(), &st) == -1) |     if (lstat(path.c_str(), &st) == -1) | ||||||
|  | @ -303,39 +318,50 @@ static void findRoots(const Path & path, bool recurseSymlinks, | ||||||
|     if (S_ISDIR(st.st_mode)) { |     if (S_ISDIR(st.st_mode)) { | ||||||
| 	Strings names = readDirectory(path); | 	Strings names = readDirectory(path); | ||||||
| 	for (Strings::iterator i = names.begin(); i != names.end(); ++i) | 	for (Strings::iterator i = names.begin(); i != names.end(); ++i) | ||||||
|             findRoots(path + "/" + *i, recurseSymlinks, roots); |             findRoots(path + "/" + *i, recurseSymlinks, ignoreUnreadable, roots); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     else if (S_ISLNK(st.st_mode)) { |     else if (S_ISLNK(st.st_mode)) { | ||||||
|         string target = readLink(path); |         Path target = absPath(readLink(path), dirOf(path)); | ||||||
|         Path target2 = absPath(target, dirOf(path)); |  | ||||||
| 
 | 
 | ||||||
|         if (isInStore(target2)) { |         if (isInStore(target)) { | ||||||
|             debug(format("found root `%1%' in `%2%'") |             debug(format("found root `%1%' in `%2%'") | ||||||
|                 % target2 % path); |                 % target % path); | ||||||
|             Path target3 = toStorePath(target2); |             Path storePath = toStorePath(target); | ||||||
|             if (store->isValidPath(target3))  |             if (store->isValidPath(storePath))  | ||||||
|                 roots.insert(target3); |                 roots.insert(storePath); | ||||||
|             else |             else | ||||||
|                 printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") |                 printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") | ||||||
|                     % path % target3); |                     % path % storePath); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         else if (recurseSymlinks) { |         else if (recurseSymlinks) { | ||||||
|             if (pathExists(target2)) |             struct stat st2; | ||||||
|                 findRoots(target2, false, roots); |             if (lstat(target.c_str(), &st2) == 0) | ||||||
|             else { |                 findRoots(target, false, ignoreUnreadable, roots); | ||||||
|                 printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target2); |             else if (ignoreUnreadable && errno == EACCES) | ||||||
|  |                 /* ignore */ ; | ||||||
|  |             else if (errno == ENOENT || errno == ENOTDIR) { | ||||||
|  |                 printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target); | ||||||
|                 /* Note that we only delete when recursing, i.e., when
 |                 /* Note that we only delete when recursing, i.e., when
 | ||||||
|                    we are still in the `gcroots' tree.  We never |                    we are still in the `gcroots' tree.  We never | ||||||
|                    delete stuff outside that tree. */ |                    delete stuff outside that tree. */ | ||||||
|                 unlink(path.c_str()); |                 unlink(path.c_str()); | ||||||
|             } |             } | ||||||
|  |             else  | ||||||
|  |                 throw SysError(format("statting `%1%'") % target); | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  | static void findRoots(PathSet & roots, bool ignoreUnreadable) | ||||||
|  | { | ||||||
|  |     Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str()); | ||||||
|  |     findRoots(rootsDir, true, ignoreUnreadable, roots); | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
| static void addAdditionalRoots(PathSet & roots) | static void addAdditionalRoots(PathSet & roots) | ||||||
| { | { | ||||||
|     Path rootFinder = getEnv("NIX_ROOT_FINDER", |     Path rootFinder = getEnv("NIX_ROOT_FINDER", | ||||||
|  | @ -410,10 +436,8 @@ void collectGarbage(GCAction action, const PathSet & pathsToDelete, | ||||||
| 
 | 
 | ||||||
|     /* Find the roots.  Since we've grabbed the GC lock, the set of
 |     /* Find the roots.  Since we've grabbed the GC lock, the set of
 | ||||||
|        permanent roots cannot increase now. */ |        permanent roots cannot increase now. */ | ||||||
|     Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str()); |  | ||||||
|     PathSet roots; |     PathSet roots; | ||||||
|     if (!ignoreLiveness) |     findRoots(roots, false); | ||||||
|         findRoots(rootsDir, true, roots); |  | ||||||
| 
 | 
 | ||||||
|     /* Add additional roots returned by the program specified by the
 |     /* Add additional roots returned by the program specified by the
 | ||||||
|        NIX_ROOT_FINDER environment variable.  This is typically used |        NIX_ROOT_FINDER environment variable.  This is typically used | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue