New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race Condition in put when use_sudo=True #1555

Closed
arnimarj opened this Issue Jan 31, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@arnimarj

arnimarj commented Jan 31, 2017

Abstract

There is a race in put, and the use_sudo=True, and writing a file with the same content from multiple connections concurrently.

Steps To Reproduce

Run this script concurrently:

import os, itertools, StringIO

from fabric.api import task, run, put, settings

@task
def megaput():
	content = '0' * 1024

	with settings(host_string = '127.0.0.1', user = os.environ['USER']):
		run('mkdir -p /tmp/megaput')

		for i in itertools.count():
			put(StringIO.StringIO(content), '/tmp/megaput/file', use_sudo = True)
			print 'copied', i, 'times'

It seems that the name of temporary file created is a hash of the host string and the file content. When puting files with the same content to the same host, there is a race for the mv operation, and with non-zero probability only the first operation will succeed.

Adding a random string to https://github.com/fabric/fabric/blob/master/fabric/sftp.py#L164 seems to make this script work without problems (e.g. hasher.update(uuid.uuid4().hex)).

@ploxiln

This comment has been minimized.

ploxiln commented Feb 4, 2017

The pseudo-random hash currently depends on the destination path, not on the file content. So this is not as bad as it sounds: this doesn't happen for putting identical files to different places simultaneously, this happens for putting multiple files (could be different content) to the same place simultaneously.

Using a random temp file makes it so that all of the mv will succeed. But also it's simpler and more effective. So I'll make a PR for it anyway ...

ploxiln added a commit to ploxiln/fab-classic that referenced this issue Feb 4, 2017

ploxiln added a commit to ploxiln/fab-classic that referenced this issue Feb 4, 2017

ploxiln added a commit to ploxiln/fab-classic that referenced this issue Feb 4, 2017

ploxiln added a commit to ploxiln/fab-classic that referenced this issue Apr 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment