0

Consider I have this piece of Java code

I wonder if there is a lockless mechanism to make the highlighted piece of code atomic. I want to avoid when someone calls fetchSomeThing(), I am in the middle of BlockX and fetchSomeThing() is looking up from new copy of A but old copies of B and C.

public class MyClass
{
   private volatile Map a, b, c; 

   public void refresh()
   {
      Map a_ = loadA();   
      Map b_ = loadB();
      Map c_ = loadC();

      //I want to make the following block atomic
      // call it Block X
      {
         this.a = a_;
         this.b = b_;
         this.c = c_;
      }
   }

   public String fetchSomeThing()
   {
       // some joint operations on this.a, this.b and this.c
   }
}

The only way I can think of is to separate this into two classes and wrap up the a, b, c in a single object.

But it is extremely cumbersome. Is there a better way?

public class MyShadowClass
{
   private Map a, b, c; 
   public MyShadowClass()
   {
      init();
   }

   public void init()
   {
      Map a_ = loadA();   
      Map b_ = loadB();
      Map c_ = loadC();

      this.a = a_;
      this.b = b_;
      this.c = c_;
   }

   public String fetchSomeThing()
   {
       // some joint operations on this.a, this.b and this.c
   }
}

public class MyClass
{
   private volatile MyShadowClass shadow; 
   public void refresh()
   {
      MyShadowClass tempShadow = new MyShadowClass();
      shadow = tempShadow; 
   }

   public String fetchSomeThing()
   {
      return shadow.fetchSomeThing();
   }

}
CuriousMind
  • 15,168
  • 20
  • 82
  • 120
  • Why do you want it lockless? It would be trivial to implement with a synchronized block. – assylias Oct 29 '14 at 15:14
  • @assylias adding `synchronized` to both the read and write methods will do it, but synchronizing that block alone will still allow the read operation to occur in the middle of the write. I would strongly advocate either locking on the class, or using a dedicated [read/write lock](http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReadWriteLock.html). Locking on the class is a small fix and simple, but the read/write lock is much more powerful. – Floegipoky Oct 29 '14 at 15:17
  • Also, just wanted to point out that by not synchronizing the `loadX()` operations you could end up with some maps that are stale and some that are current. That might not be a problem, but you should at least consider it. – Floegipoky Oct 29 '14 at 15:41
  • https://stackoverflow.com/a/16906229/715269 – Gangnus May 05 '19 at 12:10

1 Answers1

1

Simply make a compound object holding the currently valid state. You can then make use of AtomicReference to manage the current state:

 private AtomicReference<Thing> currentThing = new AtomicReference<Thing>();

 public Thing getThing() {
      while (true) {
          Thing result = currentThing.get();
          if (result != null)
              return result;
          result = buildThing();
          if (currentThing.compareAndSet(null, result);
              return result;
      }
 }

 private Thing buildThing() {
      // whatever, only important thing is that this creates
      // a new and valid instance of Thing
 }

 public void refresh() {
      Thing freshThing = buildThing();
      currentThing.set(freshThing);
 }

You may get away with simply declaring "currentThing" as normal, but volatile field if you can ensure that there is only one thread ever writing that field or you do not particularly care about getting the same instance when refresh() and initial get are executing concurrently.

Durandal
  • 19,919
  • 4
  • 36
  • 70