Is 'lock (this)' suitable for synchronizing every member function?

I have a Singleton class which is highly dependent on the XML file it uses as database. At constructor, the XML file is read and a FileSystemWatcher is watching for any further chang开发者_JAVA技巧es.

Now if any changes happens, I will read data from XML file again but at this point non of class functions should be accessible until XML file is reloaded.

My question is can I simply uses this as synchronization object (while reloading XML file) and don't make any changes to functions instead of putting a big lock on every function ?

lock(this) is best avoided - there is an edge case that some other unrelated code could also lock it, perhaps causing even a deadlock.

A better approach is:

private readonly object lockObj = new object();

And then lock(lockObj) - since this is private you avoid this edge-case.

Another strategy is to load all the data into an immutable encapsulated object; then you can swap everything by updating a single reference; and a reference assignment is guaranteed to be atomic. This allows:

private SomeModel model;
public void Refresh() {
     SomeModel newModel = new ...
     // fully load etc
     ...
     model = newModel;
}

This works best if you always take a snapshot, i.e. Instead of

var foo = model.Foo;
var bar = model.Bar;

You use:

var snapshot = model;
var foo = snapshot.Foo;
var bar = snapshot.Bar;

Since now we know that Foo and Bar are from the same model.

Don't lock on this. Ever. This is because code that you don't have control over might lock on your object and lead to a very difficult to detect deadlock scenario.

Instead, use a dedicated lock object:

private readonly object locker = new object();

Then:

lock(locker) {
   // something something
}

Since locker is marked as private, you don't have to worry about code external to your object locking your object.

If the watcher is the only writer and your objects are only readers, a ReaderWriterLock might be better. Be sure to read the remarks section.