What is the best practice for securing class threads?

Question

[Edit: It seems that the original question was about a double, not an integer. So I think this question is worth it if we change the integer to double.]

I have a rare problem reading integer properties from a class used in multiple threads that sometimes returns a null value. After initialization, the values ​​do not change.

This question points to this. The consensus is that although I am accessing an integer, I need to synchronize the properties. (Some of the original answers have been deleted). I did not select the answer there because I have not solved the problem yet.

So, I did some research on this, and I'm not sure which of the .Net 4s locking mechanisms to use or if the locks should be outside the class itself.

Here is what I thought about using:

public class ConfigInfo { private readonly object TimerIntervalLocker = new object(); private int _TimerInterval; public int TimerInterval { get { lock (TimerIntervalLocker) { return _TimerInterval; } } } private int _Factor1; public int Factor1 { set { lock (TimerIntervalLocker) { _Factor1 = value; _TimerInterval = _Factor1 * _Factor2; } } get { lock (TimerIntervalLocker) { return _Factor1; } } } private int _Factor2; public int Factor2 { set { lock (TimerIntervalLocker) { _Factor2 = value; _TimerInterval = _Factor1 * _Factor2; } } get { lock (TimerIntervalLocker) { return _Factor2; } } } } 

But I read that it is terribly slow.

Another alternative is to lock the ConfigData instance on the user side, but it seems to be a lot of work. Another alternative I've seen is Monitor.Enter and Monitor.Exit , but I think Lock is the same with less syntax.

So what is best for creating a class property stream safely?

Answer 1

but. Using locks can be slow, because it uses operating system resources, if the complexity of the properties is low, then direct locking (or interlocked.compareexchange) will be faster.

b. You must make sure that the thread does not enter the lock, and through a call from one property to another will be blocked. - If this can happen (currently this is not a problem in the code), you will need to block the thread or task.

Edit:

If the object must be installed during initialization and never changed, make it immutable (for example, .NET strings). Delete all public setters and provide the constructor with parameters to determine the initial state and, possibly, additional methods / operators for creating a new instance with a changed state (for example, var newString = "Old line" + "has been changed.";).

Answer 2

If the values ​​never change, it would be easier to just make a copy of this instance and pass each instance its own instance. No blocking is required at all.

Answer 3

I think you should rewrite your ConfigInfo class to look like this: then you cannot get problems with overflow or threads:

 public sealed class ConfigInfo { public ConfigInfo(int factor1, int factor2) { if (factor1 <= 0) throw new ArgumentOutOfRangeException("factor1"); if (factor2 <= 0) throw new ArgumentOutOfRangeException("factor2"); _factor1 = factor1; _factor2 = factor2; checked { _timerInterval = _factor1*_factor2; } } public int TimerInterval { get { return _timerInterval; } } public int Factor1 { get { return _factor1; } } public int Factor2 { get { return _factor2; } } private readonly int _factor1; private readonly int _factor2; private readonly int _timerInterval; } 

Please note that I am using checked to detect overflow problems.

Otherwise, some values ​​will give incorrect results.

For example, 57344 * 524288 will give zero in unchecked integer arithmetic (and there will be a lot of other pairs of values ​​that will give zero, and even more, that will give a negative result or a positive value that "seems" to be correct).

Answer 4

It is best, as mentioned in the comments, to make readonly properties. I thought of the following possibility:

 public class ConfigInfo { private class IntervalHolder { public static readonly IntervalHolder Empty = new IntervalHolder(); private readonly int _factor1; private readonly int _factor2; private readonly int _interval; private IntervalHolder() { } private IntervalHolder(int factor1, int factor2) { _factor1 = factor1; _factor2 = factor2; _interval = _factor1*_factor2; } public IntervalHolder WithFactor1(int factor1) { return new IntervalHolder(factor1, _factor2); } public IntervalHolder WithFactor2(int factor2) { return new IntervalHolder(_factor1, factor2); } public int Factor1 { get { return _factor1; } } public int Factor2 { get { return _factor2; } } public int Interval { get { return _interval; } } public override bool Equals(object obj) { var otherHolder = obj as IntervalHolder; return otherHolder != null && otherHolder._factor1 == _factor1 && otherHolder._factor2 == _factor2; } } private IntervalHolder _intervalHolder = IntervalHolder.Empty; public int TimerInterval { get { return _intervalHolder.Interval; } } private void UpdateHolder(Func<IntervalHolder, IntervalHolder> update) { IntervalHolder oldValue, newValue; do { oldValue = _intervalHolder; newValue = update(oldValue); } while (!oldValue.Equals(Interlocked.CompareExchange(ref _intervalHolder, newValue, oldValue))); } public int Factor1 { set { UpdateHolder(holder => holder.WithFactor1(value)); } get { return _intervalHolder.Factor1; } } public int Factor2 { set { UpdateHolder(holder => holder.WithFactor2(value)); } get { return _intervalHolder.Factor2; } } } 

Thus, your TimerInterval value is always in sync with its factors. The only problem is when some thread reads one of the properties and the other writes them from outside ConfigInfo. The first may have the wrong value, and I see no way to solve this problem without introducing a single lock root. The question is whether read operations are critical.