0

Sample code:

public class TestTestTest {

   private void setFalseFlag() {
      this.keepRunning = false;
      System.out.println("keepRunning is false");
   }

   private boolean keepRunning = true;

   public static void main(String[] args) throws InterruptedException {
      TestTestTest t = new TestTestTest();
      Thread startLoop = new Thread(() -> {
         System.out.println("before loop");
         while (t.keepRunning) {}
      });
      Thread flagChanger = new Thread(() -> {
         System.out.println("before setting flag");
         t.setFalseFlag();
      });
      startLoop.start();
      Thread.sleep(1000);
      flagChanger.start();
   }
}

This code sample starts and never finishes, because keepRunning changes are not visible to the other thread. Of course, if I use volatile or AtomicBolean for keepRunning programm starts and stops properly. But, as far as I know synchronized block or locks provides flushes to main memory on entering and on exiting or smth like that, info taken from documentation. But I can't understand how to implement it on this code sample. Looks like it is not even possible to sync on one monitor here.

Oracle Documentation

All Lock implementations must enforce the same memory synchronization semantics as provided by the built-in monitor lock:

  • A successful lock operation acts like a successful monitorEnter action
  • A successful unlock operation acts like a successful monitorExit action

JSR-133 FAQ

But there is more to synchronization than mutual exclusion. Synchronization ensures that memory writes by a thread before or during a synchronized block are made visible in a predictable manner to other threads which synchronize on the same monitor. After we exit a synchronized block, we release the monitor, which has the effect of flushing the cache to main memory, so that writes made by this thread can be visible to other threads. Before we can enter a synchronized block, we acquire the monitor, which has the effect of invalidating the local processor cache so that variables will be reloaded from main memory. We will then be able to see all of the writes made visible by the previous release.

So here is the question, am I right that it is not possible here? Or if not, how to do it properly?

Layosh M
  • 33
  • 1
  • 5
  • 1
    To not read stale values, we need to add `volatile` to `keepRunning`: `private volatile boolean keepRunning = true;`. – Turing85 Jan 01 '23 at 17:14
  • @Turing85 in theory, setting that to `volatile` does not mean an immediate effect, the specification says _when_ a _subsequent_ read observes the write, it never says when that happens. So it could be that `while (t.keepRunning)` executes some cycles, even after that flag was set to false. In practice, though, things are very different – Eugene Jan 05 '23 at 21:02
  • @Eugene I am not quite sure I can follow. The [JLS, § 17.4.5](https://docs.oracle.com/javase/specs/jls/se17/html/jls-17.html#jls-17.4.5) states: "*Two actions can be ordered by a happens-before relationship. If one action happens-before another, then the first is visible to and ordered before the second. ... A write to a volatile field (§8.3.1.4) happens-before every subsequent read of that field.*" Seems pretty clear that this *is* a hard temporal constraint: as soon as a value is written to a `volatile` field, all subsequent reads have to read the new value. – Turing85 Jan 05 '23 at 21:15
  • @Turing85 [subsequent](https://stackoverflow.com/questions/50873873/what-does-subsequent-read-mean-in-the-context-of-volatile-variables) there is kind of tricky. – Eugene Jan 05 '23 at 21:18

2 Answers2

0

"synchronized block or locks provides flushes to main memory on entering and on exiting"

You can't because you're accessing keepRunning directly. That's why volatile works, because you can put it directly on the field. If you want to use synchronized, you need a section of code that only accesses keepRunning while a lock is held (in computer science this section of code is called a "critical section").

// CHANGE
private synchronized void setFalseFlag() {
  // CHANGE
  run = false;
  System.out.println("keepRunning is false");
}

// CHANGE
private synchronized boolean keeRunning() {
   // CHANGE
   return run;
}

// CHANGE
private boolean run = true;

public static void main(String[] args) throws InterruptedException {
  TestTestTest t = new TestTestTest();
  Thread startLoop = new Thread(() -> {
     System.out.println("before loop");
     // CHANGE
     while (t.keepRunning()) {}
  });
markspace
  • 10,621
  • 3
  • 25
  • 39
-2

You can do:

private Boolean keepRunning = true; //change to object so it can be synchronized one

...

while (true)
            {
                synchronized (t.keepRunning)
                {
                    if (!t.keepRunning)
                    {
                        break;
                    }
                }
            }

But better do volatile thingy. I think the reason your version doesn't break is that java isn't guaranteed to watch variables changed from other threads unless it's volatile, therefor the while loop check is optimized to true.

siggemannen
  • 3,884
  • 2
  • 6
  • 24
  • 1
    I don't think this works. Java requires synchronization on both the read and the write, so you'd have to add a similar block where the `keepRunning = false;` is used. – markspace Jan 01 '23 at 17:26
  • Yeah, ideally it should be wrapped in both places. In this case, it's enough though – siggemannen Jan 01 '23 at 17:29
  • 4
    Just because it happens to do the desired thing in one environment, doesn’t make it a correct solution. It could break in other environments or even in the same environment after an arbitrary number of successful runs. Not only is the synchronization on the assignment missing, the assignment itself would break this, as then, the `synchronized` block would be on a *different* `Boolean` object. But correct synchronization requires not only every access to be synchronized, but also that all synchronization uses *the same object*. – Holger Jan 03 '23 at 08:07