Fix race condition when two processes create a hard link to a file in .links
This is a problem because one process may set the immutable bit before the second process has created its link. Addressed random Hydra failures such as: error: cannot rename `/nix/store/.tmp-link-17397-1804289383' to `/nix/store/rsvzm574rlfip3830ac7kmaa028bzl6h-nixos-0.1pre-git/upstart-interface-version': Operation not permitted
This commit is contained in:
		
							parent
							
								
									108e14bb18
								
							
						
					
					
						commit
						b6c989b801
					
				
					 1 changed files with 37 additions and 27 deletions
				
			
		| 
						 | 
				
			
			@ -123,9 +123,6 @@ void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
 | 
			
		|||
        
 | 
			
		||||
    printMsg(lvlTalkative, format("linking `%1%' to `%2%'") % path % linkPath);
 | 
			
		||||
 | 
			
		||||
    Path tempLink = (format("%1%/.tmp-link-%2%-%3%")
 | 
			
		||||
        % nixStore % getpid() % rand()).str();
 | 
			
		||||
 | 
			
		||||
    /* Make the containing directory writable, but only if it's not
 | 
			
		||||
       the store itself (we don't want or need to mess with its
 | 
			
		||||
       permissions). */
 | 
			
		||||
| 
						 | 
				
			
			@ -140,40 +137,53 @@ void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
 | 
			
		|||
       so make it mutable first (and make it immutable again when
 | 
			
		||||
       we're done).  We also have to make ‘path’ mutable, otherwise
 | 
			
		||||
       rename() will fail to delete it. */
 | 
			
		||||
    makeMutable(linkPath);
 | 
			
		||||
    MakeImmutable mk1(linkPath);
 | 
			
		||||
 | 
			
		||||
    makeMutable(path);
 | 
			
		||||
    MakeImmutable mk2(path);
 | 
			
		||||
 | 
			
		||||
    if (link(linkPath.c_str(), tempLink.c_str()) == -1) {
 | 
			
		||||
        if (errno == EMLINK) {
 | 
			
		||||
            /* Too many links to the same file (>= 32000 on most file
 | 
			
		||||
               systems).  This is likely to happen with empty files.
 | 
			
		||||
               Just shrug and ignore. */
 | 
			
		||||
            printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
 | 
			
		||||
            return;
 | 
			
		||||
    /* Another process might be doing the same thing (creating a new
 | 
			
		||||
       link to ‘linkPath’) and make ‘linkPath’ immutable before we're
 | 
			
		||||
       done.  In that case, just retry. */
 | 
			
		||||
    unsigned int retries = 1024;
 | 
			
		||||
    while (--retries > 0) {
 | 
			
		||||
        makeMutable(linkPath);
 | 
			
		||||
        MakeImmutable mk1(linkPath);
 | 
			
		||||
 | 
			
		||||
        Path tempLink = (format("%1%/.tmp-link-%2%-%3%")
 | 
			
		||||
            % nixStore % getpid() % rand()).str();
 | 
			
		||||
 | 
			
		||||
        if (link(linkPath.c_str(), tempLink.c_str()) == -1) {
 | 
			
		||||
            if (errno == EMLINK) {
 | 
			
		||||
                /* Too many links to the same file (>= 32000 on most
 | 
			
		||||
                   file systems).  This is likely to happen with empty
 | 
			
		||||
                   files.  Just shrug and ignore. */
 | 
			
		||||
                printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
 | 
			
		||||
                return;
 | 
			
		||||
            }
 | 
			
		||||
            if (errno == EPERM) continue;
 | 
			
		||||
            throw SysError(format("cannot link `%1%' to `%2%'") % tempLink % linkPath);
 | 
			
		||||
        }
 | 
			
		||||
        throw SysError(format("cannot link `%1%' to `%2%'") % tempLink % linkPath);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /* Atomically replace the old file with the new hard link. */
 | 
			
		||||
    if (rename(tempLink.c_str(), path.c_str()) == -1) {
 | 
			
		||||
        if (errno == EMLINK) {
 | 
			
		||||
            /* Some filesystems generate too many links on the rename,
 | 
			
		||||
               rather than on the original link.  (Probably it
 | 
			
		||||
               temporarily increases the st_nlink field before
 | 
			
		||||
               decreasing it again.) */
 | 
			
		||||
            printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
 | 
			
		||||
 | 
			
		||||
            /* Unlink the temp link. */
 | 
			
		||||
        /* Atomically replace the old file with the new hard link. */
 | 
			
		||||
        if (rename(tempLink.c_str(), path.c_str()) == -1) {
 | 
			
		||||
            if (unlink(tempLink.c_str()) == -1)
 | 
			
		||||
                printMsg(lvlError, format("unable to unlink `%1%'") % tempLink);
 | 
			
		||||
            return;
 | 
			
		||||
            if (errno == EMLINK) {
 | 
			
		||||
                /* Some filesystems generate too many links on the
 | 
			
		||||
                   rename, rather than on the original link.
 | 
			
		||||
                   (Probably it temporarily increases the st_nlink
 | 
			
		||||
                   field before decreasing it again.) */
 | 
			
		||||
                printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
 | 
			
		||||
                return;
 | 
			
		||||
            }
 | 
			
		||||
            if (errno == EPERM) continue;
 | 
			
		||||
            throw SysError(format("cannot rename `%1%' to `%2%'") % tempLink % path);
 | 
			
		||||
        }
 | 
			
		||||
        throw SysError(format("cannot rename `%1%' to `%2%'") % tempLink % path);
 | 
			
		||||
 | 
			
		||||
        break;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    if (retries == 0) throw Error(format("cannot link `%1%' to `%2%'") % path % linkPath);
 | 
			
		||||
 | 
			
		||||
    stats.filesLinked++;
 | 
			
		||||
    stats.bytesFreed += st.st_size;
 | 
			
		||||
    stats.blocksFreed += st.st_blocks;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue