0

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?

Java Addict
  • 175
  • 8
  • 1
    What is it you're trying to do? There's probably a correct way to achieve it, since your code looks highly suspect (especially the `let other threads finish` part, where you make the thread sleep. – Kayaman May 12 '14 at 06:37
  • My first guess would be the Calender usage, as it is not (the class, not its current usage) is not thread-safe, though it seems to be ok here, you are using new instances as I see. – Gábor Bakos May 12 '14 at 06:38
  • Where's `downloadInfo` being declared? – Smutje May 12 '14 at 06:38
  • You have removed too much from your code. At least the storage location of the `problems` variable and the places producing the output shown in your question are required. – Holger May 12 '14 at 13:06
  • 1
    Ditto to @Kayaman. Adding a sleep() call never _fixes_ a synchronization error: It only makes it less likely to be caught by your testing, and therefore, more likely to cause harm in the field when it finally does occur. – Solomon Slow May 12 '14 at 13:16
  • Ultimately, I am trying to use multiple threads to reduce my download time. I saw that this page mentioned [ExecutorService](http://download.oracle.com/javase/6/docs/api/java/util/concurrent/ExecutorService.html) might be able to help me: http://stackoverflow.com/questions/7502718/java-download-multiple-files-using-threads Thread1 created many threads. I told thread1 to sleep if more than 20 download threads were running. I did this to avoid overloading the CPU with threads. – Java Addict May 13 '14 at 05:46
  • As already said, you have to post the missing source code. Otherwise no one can help you. – Holger May 13 '14 at 14:35
  • 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. Thank you for the help. – Java Addict May 14 '14 at 06:09
  • Note that declaring an instance method like `populateTable()` as `synchronized` only prevents multiple threads from invoking this method *for the same instance*. You have one instance of `PopulateTable` per submitted job, so the `synchronized`modifier has no effect here. Which is not that bad as otherwise creating multiple threads was pointless. But you have to ensure that the code inside that method uses thread-safe constructs only. (And you should remove the misleading, non-effective `synchronized` modifier) – Holger May 14 '14 at 14:41

1 Answers1

2

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.

mprivat
  • 21,582
  • 4
  • 54
  • 64
  • Good call. One of Java's most obvious shortcomings. – pamphlet May 14 '14 at 06:43
  • That seems to work in the test code. Trying to port it over to the big program has not been successful yet, but I think you're right. Thanks for your assistance. – Java Addict May 15 '14 at 06:03