Files should be locked when writing to them

[JeremyTCD]

Is the following:

FileCache/src/FileCache/FileCache.cs

Lines 713 to 714 in 6f9b86c

	//write the object payload (lock the file so we can write to it and force others to wait for us to finish)
	using (FileStream stream = GetStream(cachedItemPath, FileMode.Create, FileAccess.Write))

Meant to have a true argument?

//write the object payload (lock the file so we can write to it and force others to wait for us to finish)
using (FileStream stream = GetStream(cachedItemPath, FileMode.Create, FileAccess.Write, true))

Correct me if i'm wrong, but at present, it seems like multiple threads/processes can write to a file at the same time. Additionally, if a thread attempts to read from the file while it is being written to, a partial read of the file can occur.

Same goes for:

FileCache/src/FileCache/FileCache.cs

Lines 725 to 726 in 6f9b86c

	//write the cache policy
	using (FileStream stream = GetStream(cachedPolicy, FileMode.Create, FileAccess.Write))

Edit: Minor edits to phrasing

[acarteas]

Wouldn't allowing multiple threads write access to a single file create an unpredictable race condition? For example (in pseudocode): (thread 1) //do some work foo["bar"] = 1 //do more work (thread 2) //do some work foo["bar"] = 2 //do some more work (main) thread1.start() thread2.start() thread1.join() thread2.join() print foo["bar"]; //Output unknown?

…

On Mon, Aug 6, 2018 at 10:40 PM, Jeremy Tan ***@***.***> wrote: Is the following: https://github.com/acarteas/FileCache/blob/6f9b86c5c5a7b0fcb50173e3521714 ed82609744/src/FileCache/FileCache.cs#L713-L714 Meant to have a true argument? //write the object payload (lock the file so we can write to it and force others to wait for us to finish)using (FileStream stream = GetStream(cachedItemPath, FileMode.Create, FileAccess.Write, true)) Correct me if i'm wrong, but it seems like multiple threads/processes can write to a file at the same time. Additionally, if a thread attempts to read from the file while it is being written to, a partial read of the file can occur. Same goes for: https://github.com/acarteas/FileCache/blob/6f9b86c5c5a7b0fcb50173e3521714 ed82609744/src/FileCache/FileCache.cs#L725-L726 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#19>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AP2Pozsldl0f2JoAuNwxWOm5Cq6g76lFks5uOShEgaJpZM4VxhBq> .

[acarteas]

In the example above, foo["bar"] could be 1 or it could be 2, either of which might be correct. However, with simultaneous write, we could also get 12 or 21 if both threads happen to try to write foo["bar"] at exactly the same time. While the answers 1 and 2 might be correct, I cannot see a scenario in which 12 or 21 would ever be correct. How were you thinking about the problem?

[JeremyTCD]

I think my question could have been phrased better. To be clear, we are on the same page, my issue is that files aren't locked when being written to. This is the GetStream method:

FileCache/src/FileCache/FileCache.cs

Lines 589 to 623 in 6f9b86c

	private FileStream GetStream(string path, FileMode mode, FileAccess access, bool lockfile = false)
	{
	FileStream stream = null;
	TimeSpan interval = new TimeSpan(0, 0, 0, 0, 50);
	TimeSpan totalTime = new TimeSpan();
	while (stream == null)
	{
	try
	{
	if (lockfile)
	{
	stream = File.Open(path, mode, access, FileShare.None);
	}
	else
	{
	stream = File.Open(path, mode, access, FileShare.ReadWrite);
	}
	}
	catch (IOException ex)
	{
	Thread.Sleep(interval);
	totalTime += interval;
	//if we've waited too long, throw the original exception.
	if (AccessTimeout.Ticks != 0)
	{
	if (totalTime > AccessTimeout)
	{
	throw ex;
	}
	}
	}
	}
	return stream;
	}

When the lockfile argument is false (or not provided, as is the case at present), FileShare.ReadWrite is used. As per the documentation for FileShare.ReadWrite:

Allows subsequent opening of the file for reading or writing. If this flag is not specified, any request to open the file for reading or writing (by this process or another process) will fail until the file is closed. However, even if this flag is specified, additional permissions might still be needed to access the file.

So yeah the file isn't being locked.

[acarteas]

Ah, yes I see. So the final parameter should be true in the two functions that you highlighted in your first post, correct?

…

On Tue, Aug 7, 2018 at 5:01 PM, Jeremy Tan ***@***.***> wrote: I think my question could have been phrased better. To be clear, we are on the same page, my issue is that files aren't locked when being written to. This is the GetStream method: https://github.com/acarteas/FileCache/blob/6f9b86c5c5a7b0fcb50173e3521714 ed82609744/src/FileCache/FileCache.cs#L589-L623 When the lockfile argument is false (or not provided, as is the case at present), FileShare.ReadWrite is used. As per the documentation <https://docs.microsoft.com/en-sg/dotnet/api/system.io.fileshare?view=netstandard-2.0> for FileShare.ReadWrite: Allows subsequent opening of the file for reading or writing. If this flag is not specified, any request to open the file for reading or writing (by this process or another process) will fail until the file is closed. However, even if this flag is specified, additional permissions might still be needed to access the file. So yeah the file isn't being locked. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#19 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AP2Po2uwTUJQnxu7OG0_8tj1-1mI2bblks5uOipEgaJpZM4VxhBq> .

[JeremyTCD]

There are a couple more changes, I've created a pull request with a possible solution for locking: #20.