Introduction
While working on a project that didn't use a database and only worked with files, I needed to save and read data to a file in a safe way. This is how this solution came about.
The Use Case
We need to write to a file and make sure it is safe:
private void AddLine(string line)
{
var str = File.ReadAllText("file.txt");
str += line;
File.WriteAllText("file.txt", str);
}
This sample is over simplified and can be done in other ways but in essence, we need to read a value from disk to memory and change it and save it back (for example a json config string) in a safe multithreaded manner, and file.txt may be changed in other methods too.
What We Need
In normal C# development, we use the lock
keyword to implement a read/write barrier to variables and methods, so:
object o = new object();
int i = 1;
private void Do()
{
lock(o)
{
i++;
}
}
So any call to Do()
will lock and block until the previous call is done, so we get consistent updates done.
First Attempt...
With respect to lock
, we are tempted to do:
private void AddLine(string line)
{
lock("file.txt")
{
var str = File.ReadAllText("file.txt");
str += line;
File.WriteAllText("file.txt", str);
}
}
But this will not work since the string
file.txt in not unique in memory so locking it will not guarantee consistent read/writes.
So first, we need to make sure the filename is unique across the application. The best way is to use a Dictionary<>
for speed of lookup, where we check if the filename we need is in it or not.
Improvement...
So using a Dictionary<>
gets us:
Dictionary<string, object> locks = new Dictionary<string, object>();
private object GetLock(string filename)
{
if(locks.ContainsKey(filename) == false)
locks.Add(filename, new object());
return locks[filename];
}
private void AddLine(string line)
{
lock( GetLock("file.txt") )
{
var str = File.ReadAllText("file.txt");
str += line;
File.WriteAllText("file.txt", str);
}
}
Now we have insured that there is only 1 lock object associated with a filename so the lock works as we want.
Gotchas...
A Dictionary<>
is not thread safe so we need to use ConcurrentDictionary<>
instead to make sure the adding part is done in a safe way.
Also the current implementation is done in a private manner within the current class so we need it to be available in other classes also.
In addition, the thing we need to do will be different and the above way will be a little tedious in code, so a simpler way is helpful.
The Final Code
With the above requirements, we come to the following nicer way of doing things, which uses anonymous methods and there is no lock
keyword in the code:
private void Do()
{
string fn = "file.txt";
LockManager.GetLock(fn, () => {
var str = File.ReadAllText(fn);
str += line;
File.WriteAllText(fn, str);
});
}
private void Read()
{
string str = "";
LockManager.GetLock("file.txt", () => {
str = File.ReadAllText("file.txt");
});
}
The code becomes:
class lobj
{
public int count = 0;
}
public class LockManager
{
static ConcurrentDictionary<string, lobj> _locks =
new ConcurrentDictionary<string, lobj>();
private static lobj GetLock(string filename)
{
lobj o = null;
if(_locks.TryGetValue(filename.ToLower(), out o))
{
o.count++;
return o;
}
else
{
o = new lobj();
_locks.TryAdd(filename.ToLower(), o);
o.count++;
return o;
}
}
public static void GetLock(string filename, Action action)
{
lock(GetLock(filename))
{
action();
Unlock(filename);
}
}
private static void Unlock(string filename)
{
lobj o = null;
if (_locks.TryGetValue(filename.ToLower(), out o))
{
o.count--;
if (o.count == 0)
_locks.TryRemove(filename.ToLower());
}
}
}
The code keeps track of how many calls are in the queue for accessing the filename and if the count goes down to 0
, then removes the key from the dictionary as a cleanup mechanism.
Also, the code uses lower case filename matching so it will work regardless of how you typed it in code or read from somewhere.
Final Lessons Learned
While using this code, I came across a strange situation that the system just froze and it took me a while to realize what was wrong.
The problem was that I was locking files within other locks:
LockManager.GetLock("file1.txt", () => {
LockManager.GetLock("file2.txt", () => {
});
});
So the above code had the potential of being dead-locked, which it did if somewhere else in the code I was locking file2.txt. This reminded me of the first rule you learn when using databases in general that you try to not write dead locking code and minimize the code in locks so such things are rare.
So we should do the following:
LockManager.GetLock("file1.txt", () => {
});
LockManager.GetLock("file2.txt", () => {
});
History
- First release: 7th June, 2017