Should variable be made volatile?

Question

I have a class in which there is a boolean variable called isbeingwritten. It keeps track of whether a file is being written to. A function of this class invokes a few threads which write to files. these will first check the value of the isbeingwritten variable and if it is false, set it to true and start writing, otherwise they will wait. At the end of writing they will change the value back to false. Should this variable be made volatile?

class A
{
    public boolean isbeingwrittenfalse;
    public void func()
    {
        new thread1();
        new thread2();
    }
    class thread1 implements Runnable
    {
        Thread t;
        thread1()
        {
            t=new Thread (this);
            t.start();
        }
        public void run()
        {
            while(isbeingwritten);
            isbeingwritten=true;
            //wrrite very long string
            isbeingwritten=false;
        }
    }
    class thread2 implements Runnable
    {
        Thread t;
        thread2()
        {
            t=new Thread (this);
            t.start();
        }
        public void run()
        {
            while(isbeingwritten);
            isbeingwritten=true;
            //wrrite very long string
            isbeingwritten=false;
        }
    }

The following is the correct solution

public class XSSThread implements Runnable {
    Thread xt;

    public void init() {
        xt = new Thread(this);
        xt.start();
    }

    public void run() {
        new Thread1().init();
        new Thread2().init();
    }

    public synchronized void saveToFile(String a) {
        File aFile = new File("filename.txt");
        try {
            BufferedWriter out = new BufferedWriter(new FileWriter(aFile,   aFile.exists()));
            out.write(a + "\r\n");
            out.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

public class Thread1 extends XSSThread implements Runnable{
    Thread xt1;

    public void init() {
        xt1 = new Thread(this);
        xt1.start();
    }

    public void run() {
        String a;//very long string 
       saveToFile(a);
    }
}

public class Thread2 extends XSSThread implements Runnable {
    Thread xt2;

    public void init() {
        xt2 = new Thread(this);
        xt2.start();
    }

    public void run() {
       String a;//very long string 
       saveToFile(a);
    }
}

Answer 1

Yes if you want to make your isbeingwritten be accessed by one thread at a time then you have to use volatile keyword.

The volatile modifier tells the JVM that a thread accessing the variable must always reconcile its own private copy of the variable with the master copy in memory.

Volatile variables share the visibility features of synchronized keyword oj java. This means that threads will automatically see the most up-to-date value for volatile variables.

You can use it like the.se

public class FileReadingWritingTask extends Thread {

    private volatile boolean isbeingwritten;

    public void run() {
        if (!isbeingwritten) {
            isbeingwritten = true;
            // do some stuff...

            // stuff ends
            tellMeToStop();

        }
    }

    public void tellMeToStop() {
        isbeingwritten = false;
    }
}

Answer 2

No, it is not correct, because both threads can see isbeingwritten == false before both update the field to true. Neither volatile nor AtomicBoolean solve this issue. You should use proper synchronization mechanisms:

private Object writeLock = new Object();
class thread1 implements Runnable{
    public void run(){
        synchronized (writeLock) {
            //write very long text
        }
    }
}
class thread2 implements Runnable{
    public void run(){
        synchronized (writeLock) {
            //write very long text
        }
    }
}