C# Is updating an instance by itself thread safe?

Question

I have a class called Configuration which has several properties which will be updated with values loaded from a database.
I have a data trigger event OnDataChanged in the Configuration constructor where eventhandler subscribed to a private method.
The same instance of Configuration is used by multiple threads.

If something gets updated at database table, then the OnDataChanged event gets fired and the private method is called and inside this private method I update the Configuration properties with latest data from database, so that the threads which already using the instance will have the updated data.

Is this a thread safe?
If not how can we make this thread-safe? I am not using any locks.

Edit:

Adding Sample Code:

public class Configuration
{
    private GeneralConfiguration _generalConfiguration;
    private AccountConfiguration _accountConfiguration;
    private readonly SqlTableDependency<Model> _dependency;
    public Configuration()
    {
        _dependency = new SqlTableDependency<Model>("connectionstring", "dbo.Configuration");
        _dependency.OnChanged += _dependency_OnChanged;
        _dependency.Start();
    }

    private void _dependency_OnChanged(object sender, RecordChangedEventArgs<Model> e)
    {
        Init();
    }

    private Configuration Init()
    {
        DataAccess da = new DataAccess();
        List<string> configs = da.GetConfigData();
        _generalConfiguration = JsonConvert.Deserialize<GeneralConfiguration>(configs[0]);
        _accountConfiguration = JsonConvert.Deserialize<AccountConfiguration>(configs[1]);
        return this;
    }

    public GeneralConfiguration GeneralConfiguration { get { return _generalConfiguration; } }

    public AccountConfiguration AccountConfiguration { get { return _accountConfiguration; } }
}

Answer 1

As others have commented, your code is not likely thread safe. The easiest way to make it thread-safe is create a private object and use the lock keyword-- assuming your are not using any async code.

public class Configuration {
   private object sync = new object();
   private int someSetting1;
   public int SomeSetting1 {
      get {
         lock (sync) {
            return someSetting1;
         }
      }
   }

   private decimal someSetting2;
   public decimal SomeSetting2 {
      get {
         lock (sync) {
            return someSetting2;
         }
      }
   }

   private void OnDataChanged() {
      lock (sync) {
         someSetting1 = loadFromDatabase();
         someSetting2 = loadFromDatabase();
      }
   }
}

Where this gets ugly is if you have multiple settings that need to get changed together. If the values of someSetting1 and someSetting2 are complementary to each other (such as username and password pair, for example) you have a race condition where OnDataChanged could be called in-between calls to SomeSetting1 and SomeSetting2, causing your calling code to get incongruous values. If this is a problem, then you will need to move the locking outside of the Configuration class.

One approach would simply be to lock on the configuration singleton object every time you access it-- whether updating it or reading a set of values from it. Another approach that avoids lock altogether relying instead on the atomicity of references would be for your Configuration class to store a set of values in a child class and add a public accessor to that class. The OnDataChanged would swap out the current values class, and calling code would grab whatever the current one was and then get all the values they wanted to from that immutable instance:

public class Configuration {
   public class Values {  
      public int SomeSetting1 { get; }
      public int SomeSetting2 { get; }
   }
   private Values currentValues;
   public Values CurrentValues {
      get {
         return currentValues;
      }
   }
   private void OnDataChanged() {
      Values newValues = new Values(getValuesFromDatabase());
      currentValues = newValues;
   }
}

Calling code would do something like this for sets of values:

var values = configuration.CurrentValues;
doSomething(values.SomeSetting1, values.SomeSetting2);