How to freeze a popsicle in .NET (make a class immutable)
Asked Answered
H

12

64

I'm designing a class that I wish to make readonly after a main thread is done configuring it, i.e. "freeze" it. Eric Lippert calls this popsicle immutability. After it is frozen, it can be accessed by multiple threads concurrently for reading.

My question is how to write this in a thread safe way that is realistically efficient, i.e. without trying to be unnecessarily clever.

Attempt 1:

public class Foobar
{
   private Boolean _isFrozen;

   public void Freeze() { _isFrozen = true; }

   // Only intended to be called by main thread, so checks if class is frozen. If it is the operation is invalid.
   public void WriteValue(Object val)
   {
      if (_isFrozen)
         throw new InvalidOperationException();

      // write ...
   }

   public Object ReadSomething()
   {
      return it;
   }
}

Eric Lippert seems to suggest this would be OK in this post. I know writes have release semantics, but as far as I understand this only pertains to ordering, and it doesn't necessarily mean that all threads will see the value immediately after the write. Can anyone confirm this? This would mean this solution is not thread safe (this may not be the only reason of course).

Attempt 2:

The above, but using Interlocked.Exchange to ensure the value is actually published:

public class Foobar
{
   private Int32 _isFrozen;

   public void Freeze() { Interlocked.Exchange(ref _isFrozen, 1); }

   public void WriteValue(Object val)
   {
      if (_isFrozen == 1)
         throw new InvalidOperationException();

      // write ...
   }
}

Advantage here would be that we ensure the value is published without suffering the overhead on every read. If none of the reads are moved before the write to _isFrozen as the Interlocked method uses a full memory barrier I would guess this is thread safe. However, who knows what the compiler will do (and according to section 3.10 of the C# spec that seems like quite a lot), so I don't know if this is threadsafe.

Attempt 3:

Also do the read using Interlocked.

public class Foobar
{
   private Int32 _isFrozen;

   public void Freeze() { Interlocked.Exchange(ref _isFrozen, 1); }

   public void WriteValue(Object val)
   {
      if (Interlocked.CompareExchange(ref _isFrozen, 0, 0) == 1)
         throw new InvalidOperationException();

      // write ...
   }
}

Definitely thread safe, but it seems a little wasteful to have to do the compare exchange for every read. I know this overhead is probably minimal, but I'm looking for a reasonably efficient method (although perhaps this is it).

Attempt 4:

Using volatile:

public class Foobar
{
   private volatile Boolean _isFrozen;

   public void Freeze() { _isFrozen = true; }

   public void WriteValue(Object val)
   {
      if (_isFrozen)
         throw new InvalidOperationException();

      // write ...
   }
}

But Joe Duffy declared "sayonara volatile", so I won't consider this a solution.

Attempt 5:

Lock everything, seems a bit overkill:

public class Foobar
{
   private readonly Object _syncRoot = new Object();
   private Boolean _isFrozen;

   public void Freeze() { lock(_syncRoot) _isFrozen = true; }

   public void WriteValue(Object val)
   {
      lock(_syncRoot) // as above we could include an attempt that reads *without* this lock
         if (_isFrozen)
            throw new InvalidOperationException();

      // write ...
   }
}

Also seems definitely thread safe, but has more overhead than using the Interlocked approach above, so I would favour attempt 3 over this one.

And then I can come up with at least some more (I'm sure there are many more):

Attempt 6: use Thread.VolatileWrite and Thread.VolatileRead, but these are supposedly a little on the heavy side.

Attempt 7: use Thread.MemoryBarrier, seems a little too internal.

Attempt 8: create an immutable copy - don't want to do this

Summarising:

  • which attempt would you use and why (or how would you do it if entirely different)? (i.e. what is the best way for publishing a value once that is then read concurrently, while being reasonably efficient without being overly "clever"?)
  • does .NET's memory model "release" semantics of writes imply that all other threads see updates (cache coherency etc.)? I generally don't want to think too much about this, but it's nice to have an understanding.

EDIT:

Perhaps my question wasn't clear, but I am looking in particular for reasons as to why the above attempts are good or bad. Note that I am talking here about a scenario of one single writer that writes then freezes before any concurrent reads. I believe attempt 1 is OK but I'd like to know exactly why (as I wonder if reads could be optimized away somehow, for example). I care less about whether or not this is good design practice but more about the actual threading aspect of it.


Many thanks for the response the question received, but I have chosen to mark this as an answer myself because I feel that the answers given do not quite answer my question and I do not want to give the impression to anyone visiting the site that the marked answer is correct simply because it was automatically marked as such due to the bounty expiring. Furthermore I do not think the answer with the highest number of votes was overwhelmingly voted for, not enough to mark it automatically as an answer.

I am still leaning to attempt #1 being correct, however, I would have liked some authoritative answers. I understand x86 has a strong model, but I don't want to (and shouldn't) code for a particular architecture, after all that's one of the nice things about .NET.

If you are in doubt about the answer, go for one of the locking approaches, perhaps with the optimizations shown here to avoid a lot of contention on the lock.

Hautemarne answered 3/12, 2012 at 21:0 Comment(5)
I don't think "too internal" is a good reason to avoid the MemoryBarrier. OTOH, according to Albahari, VolatileRead and VolatileWrite have a pretty straightforward implementation, deemed inefficient since it uses a full fence (MemoryBarrier) - once on each, so I don't see how your 7th implementation could be much worse than VolatileRead/Write (aside for the extra ref argument and method call etc.). In addition, the lock statement uses two MemoryBarriers, and all the Interlocked methods use a full fence as well... see: albahari.com/threading/part4.aspx#_Nonblocking_SynchronizationBreadthways
I don't know if it's just me but I think it is better to define another class to hold all relevant read-only information, and return it when needed, instead of implementing a frozen state in your "business" class in which you are not allowed to call certain methods. And if this read-only class is composed solely by data properties with private setters you won't even need to care about thread-safety, and your business class won't get polluted with frozen state verification all over it.Piderit
Why not 8? If there's a good reason why not, OK, but I'm just wondering why that's not appropriate?Sidewalk
@Kevin just because it doesn't fit existing code, plus I'm looking specifically for more info on the other ones.Hautemarne
Create the instance, configuring it using ctor parameters. Then create all the other threads.Costume
V
25

Maybe slightly off topic but just out of curiosity :) Why don't you use "real" immutability? e.g. making Freeze() return an immutable copy (without "write methods" or any other possibility to change the inner state) and using this copy instead of the original object. You could even go without changing the state and return a new copy (with the changed state) on each write operation instead (afaik the string class works this). "Real immutability" is inherently thread safe.

Visitor answered 12/12, 2012 at 17:56 Comment(4)
real immutability is the way forward. all these other solutions are absurd.Deductive
Implementing entire copies of objects which aren't mutable isn't a maintainable solution. I believe popsicles are useful for when you can't use readonly because the value of the variable isn't available during object construction. Popsicles have the benefit of making your assumptions obvious by throwing an exception if somebody tries to change that value. It's not as good as a compile-time error like 'readonly', but we should favour small, maintainable solutions. After all, immutability is about removing complexity, why would we add it back in in such a way?Menstruate
And let's not forget, any problems which arise from mutating variables will be bugs. We want to reduce the amount of these, but we don't want to burden our solutions and prevent coders from doing things. If you never assign an object more than once, isn't it immutable, in concept?Menstruate
Agree about using real immutability. If copies are a problem, you could just move the data into a frozen object, which has different accessors to the same data as the unfrozen object. The data could be a single field, and everything in that object. Means freezing/thawing is quite fast, and access should be faster due to no locks.Materi
T
8

I vote for Attempt 5, use the lock(this) implementation.

This is the most reliable means of making this work. Reader/writer locks could be employed, but to very little gain. Just go with using a normal lock.

If necessary you could improve the 'frozen' performance by first checking _isFrozen and then locking:

void Freeze() { lock (this) _isFrozen = true; }
object ReadValue()
{
    if (_isFrozen)
        return Read();
    else
        lock (this) return Read();
}
void WriteValue(object value)
{
    lock (this)
    {
        if (_isFrozen) throw new InvalidOperationException();
        Write(value);
    }
}
Toluca answered 3/12, 2012 at 21:50 Comment(11)
This doesn't really answer my question, also can you substantiate your claim that the lock approach is the most efficient?Hautemarne
I also favor locks. I'd add that locks should be preferred because the API is much simpler, than clever interlocking. In my mind, interlocking to locks is like mutexes to monitors. Also, you don't need to "lock everything", you can lock various parts of your code as needed. (i.e. convention over tricks) Sometimes you just want Sicilian ice, not a popsicle.Koweit
@Hautemarne see this post: csharptest.net/465 and the three related posts if desired.Toluca
There is no need to wait for writers to exit as I assume there is only one writer which also freezes class. I could just "go with it", but this will not further my understanding so I am looking for a more in depth answer. I'm aware one shouldn't attempt to write too much lock-free code, but I feel that this may be a situation where for example attempt 3 may be better than the lock approach.Hautemarne
@Hautemarne Re-reading the question, sorry I was a little confused. So single writer, are there concurrent reads while initializing?Toluca
No worries - intention is one writer which configures (writes/reads) then freezes, only then will there be concurrent reads.Hautemarne
@Hautemarne In that case there is not a need for any of this? At absolute most a thread-barrier in any form after construction, but I seriously doubt you need it. Even still if I were writing it I would use the fully locked approach shown in my first example :) The lock just doesn't cost enough to avoid it and it ensures the desired behavior.Toluca
Use your syncRoot code; don't lock(this). See Why is lock(this) {…} bad?Patnode
-1 Any use of lock() will prevent multiple concurrent readers. Bad if you might have more than 1 or 2 threads/tasks accessing at the same time. Really bad if you might have 32 threads all reading at the same time.Rhapsody
@Rhapsody Umm, no - the lock is only used in the writes (and Freeze), that's the whole point. It's only supposed to be called on the "original" thread anyway.Uvulitis
@Uvulitis The gate variable _isFrozen isn't under a lock or barrier, so in the situation when there are race conditions (the condition this whole system is trying to detect) the _isFrozen value might be stale in the reading thread's cache, so the else lock(this) branch could be taken even on reading threads.Rhapsody
P
5

If you really create, fill and freeze the object before showing it to other threads, then you don't need anything special to deal with thread-safety (the strong memory model of .NET is already your guarantee), so the solution 1 is valid.

But, if you give the unfrozen object to another thread (or if you are simple creating your class without knowing how users will use it) then using the version the solution that returns a new fully immutable instance is probably better. In this case, the Mutable instance is like the StringBuilder and the immutable instance is like the string. If you need an extra guarantee, the mutable instance may check its creator thread and throw exceptions if it is used from any other thread (in all methods... to avoid possible partial reads).

Pedestrianize answered 6/5, 2013 at 16:23 Comment(0)
G
2

Attempt 2 is thread safe on x86 and other processors that have a strong memory model, but how I would do it is to make thread safety the consumers problem because there is no way for you to efficiently do it within the consumed code. Consider:

if(!foo.frozen)
{
    foo.apropery = "avalue";
}

the thread saftey of the frozen property and the guard code in apropery's setter doesn't really matter because even they are perfectly thread safe you still have a race condition. Instead I would write it like

lock(foo)
{
    if(!foo.frozen)
    {
        foo.apropery = "avalue";
    }
}

and have neither of the properties inherently thread safe.

Gamete answered 3/12, 2012 at 22:39 Comment(1)
+1 : In general, an object should never have to worry about its own thread-safety. In nearly all cases, those considerations are best handled on the layer above. (A class's static data is another matter, of course.)Chrystalchryste
R
2

#1 - reader not threadsafe - I believe problem would be in reader side, not writer (code not shown)
#2 - reader not threadsafe - same as #1
#3 - promising, read check can be optimized out for most cases (when CPU caches are in sync)

Attempt 3:

Also do the read using Interlocked.

public class Foobar {

  private object _syncRoot = new object();
  private int _isFrozen = 0; // perf compiler warning, but training code, so show defaults

  // Why Exchange to 1 then throw away result.  Best to just increment.
  //public void Freeze() { Interlocked.Exchange(ref _isFrozen, 1); }
  public void Freeze() { Interlocked.Increment(ref _isFrozen); }

  public void WriteValue(Object val) {
       // if this core can see _isFrozen then no special lock or sync needed
       if (_isFrozen != 0)
           throw new InvalidOperationException();

       lock(_syncRoot) {
           if (_isFrozen != 0)
               throw new InvalidOperationException(); // the 'throw' is 100x-1000x more costly than the lock, just eat it
           _val = val;
       }

  }

  public object Read() {
       // frozen is one-way, if one-way state has been published 
       // to my local CPU cache then just read _val.  
       // There are very strange corner cases when _isFrozen and _val fields are in 
       // different cache lines, but should be nearly impossible to hit unless
       // dealing with very large structs (make it more likely to cross 
       // 4k cache line).
       if (_isFrozen != 0) 
           return _val;

       // else
       lock(_syncRoot) { // _isFrozen is 0 here
           if (_isFrozen != 0) // if _isFrozen is 1 here we just collided with writer using lock on other thread, or our CPU cache was out of sync and lock() forced the dirty cache line to be read from main memory
              return _val;

           throw new InvalidOperationException(); // throw is 100x-1000x more expensive than lock, eat the cost of lock
       }          
  }

}

Joe Duffy's post about 'volatile is dead' is, I think, in the context of his next-gen CLR/OS architecture and for CLR on ARM. Those of us doing multi-core x64/x86 I think volatile is fine. If perf is the primary concern I suggest you measure the code above and compare it to volatile.

Unlike other folks posting answers I wouldn't jump straight to lock() if you have lots of readers (3 or more threads likely to read the same object at the same time). But in your sample you mix perf-sensitive question with exceptions when a collision happens, which doesn't make much sense. If you're using exceptions, then you can also use other higher-level constructs.

If you want complete safety but need to optimize for lots of concurrent readers change lock()/Monitor to ReaderWriterLockSlim.

.NET has new primitives to handle publishing values. Take a look at Rx. It can be very fast and lockless for some cases (I think they use optimizations similar to above).

If written multiple times but only one value is kept - in Rx that is "new ReplaySubject(bufferSize: 1)". If you try it you might be surprised how fast it. At the same time I applaud your attempt to learn this level of detail.

If you want to go lockless get over your distaste for Thread.MemoryBarrier(). It is extremely important. But it has the same gotchas as volatile as described by Joe Duffy - it was designed as a hint to the compiler & CPU to prevent reordering of memory reads (which take a long time in CPU terms, so they are aggressively reordered when there are no hints present). When this reordering is combined with CLR constructs like auto-inline of functions and you can see very surprising behavior at the memory & register level. MemoryBarrier() just disables those single-threaded memory access assumptions that CPU and CLR use most of the time.

Rhapsody answered 13/12, 2012 at 11:27 Comment(3)
Given the strong .NET memory model and that writes have release semantics am I correct in assuming this means those writes are effectively "flushed" to memory in such a way that all concurrent reads that come after can see the update? I am curious about this because I have an application that initializes and becomes effectively read only after (e.g. a web application which starts and then serves requests). None of the reads for the web request are under locks, and it seems to me they don't need to be. This would point me to option 1. I do see that your version is efficient if using locks.Hautemarne
I believe the behavior you're describing is only for volatile reads and writes, not normal ones. If all writes were immediately propagated to all other cores that would be massively inefficient, not just in perf, but in power use. This might also be hard to test, because memory controllers are so insanely fast that it might be hard to catch a stale read from another core (except when under load, but that's when it counts most).Rhapsody
"However, in the .NET model, reads can be reordered, which means the compiler could eagerly try to register myValue, effectively transforming the code into the following: ..." msdn.microsoft.com/en-us/magazine/cc163715.aspx#S10 If all the other problems are solved and the only one you have left is the reodering of reads, then look into MemoryBarrier().Rhapsody
T
2

Perhaps my question wasn't clear, but I am looking in particular for reasons as to why the above attempts are good or bad. Note that I am talking here about a scenario of one single writer that writes then freezes before any concurrent reads. I believe attempt 1 is OK but I'd like to know exactly why (as I wonder if reads could be optimized away somehow, for example). I care less about whether or not this is good design practice but more about the actual threading aspect of it.

Ok, now I better understand what you are doing and looking for in a response. Allow me to elaborate on my previous answer promoting the use of locks by first addressing each of your attempts.

Attempt 1:

The approach of using a simple class that has no synchronization primitives of any form is entirely viable in your example. Since the 'authoring' thread is the only thread having access to this class during it's mutating state this should be safe. If an only if another thread has the potential to access before the class is 'frozen' would you need to provide synchronization. Essentially, it's not possible for a thread to have a cache of something it has never seen.

Aside from a thread having a cached copy of the internal state of this list there is one other concurrency issue that you should be concerned with. You should consider write reordering by the authoring thread. You example solution doesn't have enough code for me to address this, but the process of handing this 'frozen' list to another thread is the heart of the issue. Are you using Interlocked.Exchange or writing to a volatile state?

I still advocate that is not the best approach simply because there is no guarantee that another thread has not seen the instance while it's mutating.

Attempt 2:

While attempt 2 should not be used. If you are using atomic writes to a member, one should also use atomic reads. I would never recommend one without the other as without both reads and writes being atomic you haven't gained anything. The correct application of atomic reads and writes is your 'Attempt 3'.

Attempt 3:

This will guarantee an exception is thrown if a thread has attempted to mutate an frozen list. However it makes no assertion that a read is only acceptable on a frozen instance. This, IMHO, is just as bad as accessing our _isFrozen variable with atomic and non-atomic accessors. If you are going to say that it's important to safeguard writes, then you should always safeguard reads. One without the other is just 'odd'.

Overlooking my own feeling towards writing code that gaurds writes but not reads this is an acceptable approach given your specific uses. I have one writer, I write, I freeze, then I make it available to readers. Under this scenario you code works correctly. You rely on the atomic operation on the set of _isFrozen to provide the required memory barrier prior to handing the class to another thread.

In a nutshell this approach works, but again if a thread has an instance that is not frozen it's going to break.

Attempt 4:

While at heart this is nearly the same as attempt 3 (given one writer) there is one big difference. In this example, if you check _isFrozen in the reader then every access will require a memory barrier. This is unnecessary overhead once the list is frozen.

Still this has the same issue as Attempt 3 in that no assertions are made about the state of _isFrozen during the read so the performance should be identical in your example usage.

Attempt 5:

As I said this is my preference given the modification to read as appears in my other answer.

Attempt 6:

Is essentially the same as #4.

Attempt 7:

You could solve your specific needs with a Thread.MemoryBarrier. Essentially using the code from Attempt 1, you create the instance, call Freeze(), add your Thread.MemoryBarrier, and then share the instance (or share it within a lock). This should work great, again only under your limited use case.

Attempt 8:

Without knowing more about this, I can't advise on the cost of the copy.

Summary

Again I prefer using a class that has some threading guarantee or none at all. Creating a class that is only 'partially' thread safe is, IMO, dangerous.

In the words of a famous jedi master:

Either do or do not there is no try.

The same goes for thread safety. The class should either be thread safe or not. Taking this approach you are left with either using my augmentation of Attempt 5, or using Attempt 7. Given the choice, I would never recommend #7.

So my recommendation stands firmly behind a completely thread-safe version. The performance cost between the two is so infinitesimally small it's almost non-existent. The reader threads will never hit the lock simply because of your usage scenario of having a single writer. Yet, if they do, proper behavior is still a certainty. Thus as your code changes over time and suddenly your instance is being shared prior to being frozen you don't wind up with race condition that crashes your program. Thread safe, or not, don't be half-in or you wind up with nasty surprise someday.

My preference is all classes shared by more than one thread are one of two types:

  1. Completely immutable.
  2. Completely Thread-safe.

Since a popsicle list is not immutable by design it does not fit #1. Therefore if you are going to share the object across threads it should fit #2.

Hopefully all this ranting further explains my reasoning :)

_syncRoot

Many people have noticed that I skipped the use of a _syncRoot on my locking implementation. While the reasons to use _syncRoot are valid they are not always necessary. In your example usage where you have a single writer the use of lock(this) should suffice nicely without adding another heap allocation for _syncRoot.

Toluca answered 13/12, 2012 at 20:55 Comment(1)
The use of _syncRoot isn't to protect you from youself, it is to protect you from rouge libraries that have access to your object, and therefore have the ability to deadlock you without warning by locking your object from some other thread. .NET won't do this (well, not any more), but other bad libraries could.Rhapsody
M
1

Is the thing constructed and written to, then permanently frozen and read multiple times?

Or do you freeze and unfreeze and refreeze it multiple times?

If it's the former, then perhaps the "is frozen" check should be in the reader method not the writer method (to prevent it reading before it's frozen).

Or, if it's the latter, then the use case you need to beware of is:

  • Main thread invokes the writer method, finds that it's not frozen, and therefore begins to write
  • Before the write has finished, someone tries to freeze the object and then reads from it, while the other (main) thread is still writing

In the latter case, Google shows a lot of results for multiple reader single writer which you might find interesting.

Mays answered 3/12, 2012 at 21:49 Comment(5)
Just to be clear the idea is permanent freezing. The isFrozen check is in the correct place as we want to prevent writing after the class is frozen. If it is not frozen the assumption is there is a single writer who can read/write freely.Hautemarne
@Hautemarne If it's permanently frozen, and if the same (main) thread is the only thread which calls the Freeze method and the WriteValue method, then your Attempt #1 is the simplest and cheapest solution, and is sufficient (assuming that there's another mechanism to prevent its being read before its frozen, for example, not exposing/publishing the instance to the reader threads until after it's frozen).Mays
that's my thinking exactly, can you elaborate as to why this is the case?Hautemarne
@Hautemarne Because the same (main) thread is setting the frozen flag, and later reading from it. The other (reader) threads are therefore irrelevant. Any thread (e.g. the main thread) which writes a data value (e.g. the frozen flag) can safely expect to be able to read that data value later, without doing anything special.Mays
+1. @Marcus, assuming no other threads are aware of the object before it is frozen I believe you don't need any locking. I don't think there is a thread safe way to notify another thread about existance of new entity without memory barrier (this maybe the actual question you are asking). As result all other threads will get consistent frozen state (since thy will only be able to learn about new object and use it after memory barrier).Schoenberg
C
1

In general, each mutable object should have precisely one clearly-defined "owner"; shared objects should be immutable. Popsicles should not be accessible by multiple threads until after they are frozen.

Personally, I don't like forms of popsicle immunity with an exposed "freeze" method. I think a cleaner approach is to have AsMutable and AsImmutable methods (each of which would simply return the object unmodified when appropriate). Such an approach can allow for more robust promises about immutability. For example, if an "unshared mutable object" is being mutated while its AsImmutable member is being called (behavior which would be contrary to the object being "unshared"), the state of the data in the copy may be indeterminate, but whatever was returned would be immutable. By contrast, if one thread froze an object and then assumed it was immutable while another thread was writing to it, the "immutable" object could end up changing after it was frozen and its values were read.

Edit

Based on further description, I would suggest having code which writes to the object do so within a monitor lock, and having the freeze routine look something like:

public Thingie Freeze(void) // Returns the object in question
{
  if (isFrozen) // Private field
    return this;
  else
    return DoFreeze();
}

Thingie DoFreeze(void)
{
  if (Monitor.TryEnter(whatever))
  {
    isFrozen = true;
    return this;
  }
  else if (isFrozen)
    return this;
  else
    throw new InvalidOperationException("Object in use by writer");
}

The Freeze method may be called any number of times by any number of threads; it should be short enough to be inlined (though I haven't profiled it), and should thus take almost no time to execute. If the first access of the object in any thread is via the Freeze method, that should guarantee proper visibility under any reasonable memory model (even if the thread didn't see the updates to the object performed by the thread which created and originally froze it, it would perform the TryEnter, which would guarantee a memory barrier, and after that failed it would notice that the object was frozen and return it.

If code which is going to write the object acquires the lock first, an attempt to write to a frozen object could deadlock. If one would rather have such code throw an exception, one use TryEnter and throw an exception if it can't get the lock.

The object used for locking should be something which is exclusively held by the object to be frozen. If the object to be frozen doesn't hold a purely-private reference to anything, one could either lock on this or create a private object purely for locking purposes. Note that it is safe to abandon 'entered' monitor locks without cleanup; the GC will simply forget about them, since if no references exist to a lock there's no way anybody will ever care (or could even ask) whether the lock was entered at the time it was abandoned.

Caesura answered 3/12, 2012 at 23:17 Comment(1)
Yes the intention is for the popsicle to be accessible by multiple threads only after it is frozen (so only one thread will write to it and then freeze it). I appreciate your solution, but I don't think it's an approach that works for me at the moment. Also, I am looking for reasons why attempt X would work/not work in order to get a better understanding.Hautemarne
H
1

I am not sure in terms of cost how the following approach will do, but it is a bit different. Only initially if there are multiple threads trying to write value simultaneously will they encounter locks. Once it is frozen all later calls will get the exception directly.

Attempt 9:

public class Foobar
{
    private readonly Object _syncRoot = new Object();
    private object _val;
    private Boolean _isFrozen;

    private Action<object> WriteValInternal;

    public void Freeze() { _isFrozen = true; }

    public Foobar()
    {
        WriteValInternal = BeforeFreeze;
    }

    private void BeforeFreeze(object val)
    {
        lock (_syncRoot)
        {
            if (_isFrozen == false)
            {
                //Write the values....
                _val = val;
                //...
                //...
                //...
                //and then modify the write value function
                WriteValInternal = AfterFreeze;
                Freeze();
            }
            else
            {
                throw new InvalidOperationException();
            }
        }
    }

    private void AfterFreeze(object val)
    {
        throw new InvalidOperationException();
    }

    public void WriteValue(Object val)
    {
        WriteValInternal(val);
    }

    public Object ReadSomething()
    {
        return _val;
    }
}
Hanoverian answered 6/12, 2012 at 23:49 Comment(2)
IIUC I think there is a small window when a reader thread could get null _val after writer thread updated it (if ReadSomething() is inlined into another function, and memory read is reordered to top of the larger function ... this is what MemoryBarrier() is for).Rhapsody
Also, why is changing an Action<object> variable more threadsafe than changing a bool? The 2nd writer could have the old copy of WriteValInternal in its cache after the 1st writer finishes, right? Same reason as _val - a reordered memory read. MemoryBarrier() would eliminate some risk, but not all, IIUC.Rhapsody
A
0

Have you checked out Lazy

http://msdn.microsoft.com/en-us/library/dd642331.aspx

which uses ThreadLocal

http://msdn.microsoft.com/en-us/library/dd642243.aspx

And actually looking further there is a Freezable class...

http://msdn.microsoft.com/en-us/library/vstudio/ms602734(v=vs.100).aspx

Archimage answered 13/12, 2012 at 19:37 Comment(7)
System.Windows.Freezable is a UI construct, and I don't think it helps the OP's understand low-level memory models and race conditions.Rhapsody
It may not be entirely acceptable but there is really nothing stopping one from using it technically.Archimage
It doesn't apply to this context. He's looking at low-level memory models, not high-level UI constructs. There are high-level constructs availble in .NET that do apply here, like Rx Subject<T>, so I would recommend readers to stay away from System.Windows and look elsewhere.Rhapsody
Well thanks for Nit Picking. Any reason why Lazy is not sufficient? It could easily be done in a few lines with Lazy.Archimage
It can't be set multiple times with Lazy.Rhapsody
Then you're throwing away all the thread safety benefits of Lazy<>, what's the point? Subject<T> was designed to be a changing stream of values (and be thread safe).Rhapsody
Then why is there Freeze without Unfreeze... by nature Frozen things thaw... The thread safety helps maintain the requirement of being frozen buddy. tsk tsk.Archimage
B
0

you may achieve this using POST Sharp

take one interface

public interface IPseudoImmutable
{

    bool IsFrozen { get; }


    bool Freeze();
}

then derive your attribute from InstanceLevelAspect like this

 /// <summary>
/// implement by divyang
/// </summary>
[Serializable]
[IntroduceInterface(typeof(IPseudoImmutable),
    AncestorOverrideAction = InterfaceOverrideAction.Ignore, OverrideAction = InterfaceOverrideAction.Fail)]
public class PseudoImmutableAttribute : InstanceLevelAspect, IPseudoImmutable
{

    private volatile bool isFrozen;

    #region "IPseudoImmutable"


    [IntroduceMember]
    public bool IsFrozen
    {
        get
        {
            return this.isFrozen;
        }
    }


    [IntroduceMember(IsVirtual = true, OverrideAction = MemberOverrideAction.Fail)]
    public bool Freeze()
    {
        if (!this.isFrozen)
        {
            this.isFrozen = true;
        }

        return this.IsFrozen;
    }

    #endregion


    [OnLocationSetValueAdvice]
    [MulticastPointcut(Targets = MulticastTargets.Property | MulticastTargets.Field)]
    public void OnValueChange(LocationInterceptionArgs args)
    {
        if (!this.IsFrozen)
        {
            args.ProceedSetValue();
        }
    }
}


public class ImmutableException : Exception
{
    /// <summary>
    /// The location name.
    /// </summary>
    private readonly string locationName;

    /// <summary>
    /// Initializes a new instance of the <see cref="ImmutableException"/> class.
    /// </summary>
    /// <param name="message">
    /// The message.
    /// </param>
    public ImmutableException(string message)
        : base(message)
    {
    }


    public ImmutableException(string message, string locationName)
        : base(message)
    {
        this.locationName = locationName;
    }


    public string LocationName
    {
        get
        {
            return this.locationName;
        }
    }
}

then apply in your class like this

    [PseudoImmutableAttribute]
public class TestClass
{



    public string MyString { get; set; }




    public int MyInitval { get; set; }
}

then run it in multi thread

 /// <summary>
/// The program.
/// </summary>
public class Program
{
    /// <summary>
    /// The main.
    /// </summary>
    /// <param name="args">
    /// The args.
    /// </param>
    public static void Main(string[] args)
    {
        Console.Title = "Divyang Demo ";
        var w = new Worker();
        w.Run();
        Console.ReadLine();
    }
}


internal class Worker
{

    private object SyncObject = new object();


    public Worker()
    {
        var r = new Random();
        this.ObjectOfMyTestClass = new MyTestClass { MyInitval = r.Next(500) };
    }


    public MyTestClass ObjectOfMyTestClass { get; set; }


    public void Run()
    {

        Task readWork;

        readWork = Task.Factory.StartNew(

            action: () =>
                {
                    for (;;)
                    {
                        Task.Delay(1000);
                        try
                        {
                            this.DoReadWork();
                        }
                        catch (Exception exception)
                        {
                            // Console.SetCursorPosition(80,80);
                            // Console.SetBufferSize(100,100);
                            Console.WriteLine("Read Exception : {0}", exception.Message);
                        }
                    }
                    // ReSharper disable FunctionNeverReturns
                });


        Task writeWork;


        writeWork = Task.Factory.StartNew(

            action: () =>
                {
                    for (int i = 0; i < int.MaxValue; i++)
                    {
                        Task.Delay(1000);
                        try
                        {
                            this.DoWriteWork();
                        }
                        catch (Exception exception)
                        {
                            Console.SetCursorPosition(80, 80);
                            Console.SetBufferSize(100, 100);
                            Console.WriteLine("write Exception : {0}", exception.Message);
                        }

                        if (i == 5000)
                        {

                            ((IPseudoImmutable)this.ObjectOfMyTestClass).Freeze();

                        }
                    }
                });

        Task.WaitAll();
    }

    /// <summary>
    /// The do read work.
    /// </summary>
    public void DoReadWork()
    {
        // ThreadId  where reading is done
        var threadId = System.Threading.Thread.CurrentThread.ManagedThreadId;

        // printing on screen
        lock (this.SyncObject)
        {
            Console.SetCursorPosition(0, 0);
            Console.SetBufferSize(290, 290);
            Console.WriteLine("\n");
            Console.WriteLine("Read Start");
            Console.WriteLine("Read => Thread Id: {0} ", threadId);
            Console.WriteLine("Read => this.objectOfMyTestClass.MyInitval: {0} ", this.ObjectOfMyTestClass.MyInitval);
            Console.WriteLine("Read => this.objectOfMyTestClass.MyString: {0} ", this.ObjectOfMyTestClass.MyString);


            Console.WriteLine("Read End");
            Console.WriteLine("\n");
        }
    }

    /// <summary>
    /// The do write work.
    /// </summary>
    public void DoWriteWork()
    {
        // ThreadId  where reading is done
        var threadId = System.Threading.Thread.CurrentThread.ManagedThreadId;

        // random number generator
        var r = new Random();
        var count = r.Next(15);

        // new value for Int property
        var tempInt = r.Next(5000);
        this.ObjectOfMyTestClass.MyInitval = tempInt;

        // new value for string Property
        var tempString = "Randome" + r.Next(500).ToString(CultureInfo.InvariantCulture);

        this.ObjectOfMyTestClass.MyString = tempString;


        // printing on screen
        lock (this.SyncObject)
        {
            Console.SetBufferSize(290, 290);
            Console.SetCursorPosition(125, 25);

            Console.WriteLine("\n");
            Console.WriteLine("Write Start");
            Console.WriteLine("Write => Thread Id: {0} ", threadId);
            Console.WriteLine("Write => this.objectOfMyTestClass.MyInitval: {0} and New Value :{1} ", this.ObjectOfMyTestClass.MyInitval, tempInt);
            Console.WriteLine("Write => this.objectOfMyTestClass.MyString: {0} and New Value :{1} ", this.ObjectOfMyTestClass.MyString, tempString);
                          Console.WriteLine("Write End");
            Console.WriteLine("\n");
        }
    }
}





but still it will allow you to change property like array ,list . but if you apply more login in that then it may work for all type of property and field
Brynnbrynna answered 20/12, 2013 at 16:11 Comment(0)
M
0

I'd do something like this, inspired by C++ movable types. Just remember not to access the object after Freeze/Thaw.

Of course, you can add a _data != null check/throw if you want to be clear about why the user gets an NRE if accessing after thaw/freeze.

public class Data 
{
   public string _foo;
   public int _bar;
}

public class Mutable 
{
   private Data _data = new Data();

   public Mutable() {}

   public string Foo { get => _data._foo; set => _data._foo = value; }
   public int Bar { get => _data._bar; set => _data._bar = value; }

   public Frozen Freeze() 
   { 
      var f = new Frozen(_data);
      _data = null;
      return f;
   }
}

public class Frozen 
{
   private Data _data;

   public Frozen(Data data) => _data = data;              

   public string Foo => _data._foo;  
   public int Bar => _data._bar;

   public Mutable Thaw() 
   { 
      var m = new Mutable(_data);
      _data = null;
      return m;
   }
}
Materi answered 19/2, 2018 at 20:4 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.