How do I solve this memory consistency error?

Question

I have a multithreaded Java program which is getting memory consistency errors. I've naively tried to solve the problem by declaring all my variables volatile and methods synchronized, but that has not worked.

Sometimes, the memory from the previous object shows up in other new objects. Here is real world output:

<Thread-13> OptionRow (AMGN_062714P116) expDate set to 2014-06-27
Thread-9 2) OFT-Thread (AIG_052314C50) populateTable expDate = 2014-06-27 year=2014 month=5 day=23
<Thread-9> OptionRow (AIG_052314C50) expDate set to 2014-05-23

You can see that Thread 9 set the expDate to the same value that Thread 13 set the value. That was wrong though, and somehow, the correct value caught up. It's tough to make this a small code snippet, so I'll do my best to accurately describe the code sequence while keeping it short.

edit: I modified the code per request (original code was too short)

There's still some stuff missing from this code. I cannot include it because it would allow you to connect to TDAmeritrade's API servers. I can create randomly generated dates if that would be better? I know this will not compile for you. I'm still learning the protocol for this site. Let me know if this is wrong.

Also, when I ran this as a single thread, it worked. There were no memory errors. So, it is definitely the multithreading. This version is different from what I originally posted. I hoped that using the ExecutorService thread pool would help. It does not help.

import java.util.Iterator;
import java.util.TreeSet;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

import broker.Root;
import java.util.ArrayList;
import java.util.GregorianCalendar;
import networking.ParseBinaryOptionChain;

public class StackOverflowCode {
    public StackOverflowCode () {
        Root root = new Root();

        MyStatement myStatement = new MyStatement (root);
        root.setStatement(myStatement);

        networking.TDAConnection tdaConn = new networking.TDAConnection(root);
        tdaConn.login();
        root.setTDAConnection(tdaConn);

        Table table = new Table(root);
        new Thread(table).start();        
    }

    public static void main(String[] args){
        new StackOverflowCode();
    }
}

class Table implements Runnable {
    Root root;
    private ArrayList <OptionRow> optionList;
    public Table(Root r){
        // guess initial list size is 125,000 symbols
        optionList = new ArrayList<>(125000); 
        root = r;
    }

    @Override
    public void run() {
        getUnderlyingList(); 
    }

    public void addAll(ArrayList options) {
        optionList.addAll(options);
    }

    private void getUnderlyingList() {
        try {
            final TreeSet dbStocks = (TreeSet) root.getMyStatement().getStockSymbolsFromOptionsFilterDatabase();
            ExecutorService pool = Executors.newFixedThreadPool(10);
            String stock = null;
            for (Iterator it = dbStocks.iterator(); it.hasNext(); stock = (String) it.next()) {
                pool.submit(new Download(root, stock, this));
            }
            pool.shutdown();
            pool.awaitTermination(Long.MAX_VALUE, TimeUnit.MILLISECONDS);
        } catch (InterruptedException ex) {
            ex.printStackTrace();
        }
    }
}

class Download implements Runnable {

    private String stock;
    private Root root;
    private Table parent;

    public Download(Root root, String stock, Table parent) {
        this.root = root;
        this.stock = stock;
        this.parent = parent;
    }

    @Override
    public void run() {
        PopulateTable pop = new PopulateTable(root, stock, parent);
        pop.populateTable();
    }
}

class PopulateTable {
    private String stockSymbol;
    private Root root;
    private Table parent;
    private ArrayList <OptionRow> optionList;
    private ParseBinaryOptionChain pBOC;
    private volatile OptionRow option;
    private volatile GregorianCalendar expDate;

    public PopulateTable(Root root, String stockSymbol, Table parent) {
        this.root = root;
        this.stockSymbol = stockSymbol;
        this.parent = parent;
        optionList = new ArrayList<>();
    }

    public synchronized void populateTable() {
        try {
            // Download all the options associated with the stock symbol
            // and parse them directly from the Internet.
            pBOC = root.getTDAConnection().requestBinaryOptionChain(stockSymbol);

            root.getMyStatement().setLastStockPrice(stockSymbol, pBOC.getsLast());
            root.getMyStatement().setCompanyName(stockSymbol, new networking.YahooConnect(root).symbolLookup(stockSymbol));

            // Here's where we will be going through each option. Collect the info
            // we are interested in here. Put the info in the options table. 
            while (pBOC.next()&&root.buttonIsOnline()){ 

                boolean isCall = false;
                if (pBOC.getPutCallIndicator()=='C') 
                    isCall = true;

                String date = pBOC.getODate();
                expDate = new GregorianCalendar (
                        Integer.parseInt(date.substring(0, 4)),
                        (Integer.parseInt(date.substring(4, 6))-1),
                        Integer.parseInt(date.substring(6, 8)));

                System.out.println ("<"+Thread.currentThread().getName()+"> ("+pBOC.getOSymbol()+") populateTable expDate = "
                        +root.getHelp().getSdfYearMonthDay().format(expDate.getTime())
                        +" year="+Integer.parseInt(date.substring(0, 4))
                        +" month="+Integer.parseInt(date.substring(4, 6))
                        +" day="+ Integer.parseInt(date.substring(6, 8)));

                System.out.println (Thread.currentThread().getName()+" 2) OFT-Thread ("+pBOC.getOSymbol()+") populateTable expDate = "
                        +root.getHelp().getSdfYearMonthDay().format(expDate.getTime())
                        +" year="+Integer.parseInt(date.substring(0, 4))
                        +" month="+Integer.parseInt(date.substring(4, 6))
                        +" day="+ Integer.parseInt(date.substring(6, 8)));

                option = new OptionRow(root, expDate, new GregorianCalendar());
                optionList.add(option);
            }
            // add all the options to the parent table
            parent.addAll(optionList);
            // Saves all the downloaded info to the database
            root.getMyStatement().insertOptionsInfoForFilter(optionList);
        } catch (Exception ex) {
            ex.printStackTrace();
        }
    }
}

class OptionRow {
   private Root root;
   private volatile GregorianCalendar expDate;

   public OptionRow (Root root, GregorianCalendar expDate, Object... o) {
       this.root = root;
       this.expDate = expDate;
   }
}

I would really love it if someone could tell my why the errors are happening and how to fix it?

Answer 1

I'd think the problem is simply that Root.getHelp().getSdfYearMonthDay() is returning a shared date formatter. These objects aren't thread safe and typically, sharing those leads to formatting another thread's date.

Instead of

root.getHelp().getSdfYearMonthDay()

Replace with:

new SimpleDateFormat("yyyy-MM-dd")

And see if the problem persists. I'll bet it's gone.