File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Threads and Synchronization and the fly likes simulation of trading application, thread safety issues ? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of JavaScript Promises Essentials this week in the JavaScript forum!
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "simulation of trading application, thread safety issues ?" Watch "simulation of trading application, thread safety issues ?" New topic
Author

simulation of trading application, thread safety issues ?

Edward Chen
Ranch Hand

Joined: Dec 23, 2003
Posts: 798
I am learning Thread, so I write an application to test my understanding.

Here is a simulation of trading application. The vendor, like Bloomberg, send out the latest market data of a security. My application receives it and save it into queue. And threads will process it one by one. After this action, it will save the updated security into another queue. Then threads will update GUI based these data.

This is just a simulation, some coding is missing, for example, shut down the thread pool. security price are randomly generated.

I read the log, one thing is for sure, all feeds are NOT lost. But I am not sure my design is good, or has some another thread safety issues.

Below is the fully runnable coding. Please advise.

Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

I didn't look through all your code. But here are the suggestions I came up for what I did look at:

1) You have a ThreadPool designed to update the GUI. I know you are not actually updating a GUI in the code you provided - but you should only update a GUI in a single thread - specifically the Event Dispatch Thread. This is because most GUI components are NOT thread safe. There are tools (SwingWorker and SwingUtilities' invoke...() methods for example) which will do the job for you.

2) You have a section of your code where you double check the value of a reference (inside updateFeed):

First - If you needed to use double checking on this piece of code this would not work. With code re-ordering allowed both if conditions could be checked prior to the synchronized block. The only way to ensure that doesn't happen is to make the security reference volatile.

Second - You do not need double checking here. The security reference is local, which means only the current thread has access to it (the reference. Other threads may have access to the Object it points to, but for null checking that is unimportant.). If it is not-null once it will be not-null any other time you check unless you explicitly put code in there to make it null. Additionally, the security reference is final. If the security reference is non-null once it can not be changed to null.


Steve
Edward Chen
Ranch Hand

Joined: Dec 23, 2003
Posts: 798
Steve Luke wrote:I didn't look through all your code. But here are the suggestions I came up for what I did look at:

1) You have a ThreadPool designed to update the GUI. I know you are not actually updating a GUI in the code you provided - but you should only update a GUI in a single thread - specifically the Event Dispatch Thread. This is because most GUI components are NOT thread safe. There are tools (SwingWorker and SwingUtilities' invoke...() methods for example) which will do the job for you.

2) You have a section of your code where you double check the value of a reference (inside updateFeed):

First - If you needed to use double checking on this piece of code this would not work. With code re-ordering allowed both if conditions could be checked prior to the synchronized block. The only way to ensure that doesn't happen is to make the security reference volatile.

Second - You do not need double checking here. The security reference is local, which means only the current thread has access to it (the reference. Other threads may have access to the Object it points to, but for null checking that is unimportant.). If it is not-null once it will be not-null any other time you check unless you explicitly put code in there to make it null. Additionally, the security reference is final. If the security reference is non-null once it can not be changed to null.


Thanks for your reply. I agree with the first point.

Regarding the double check,
1. I should modify it to resultQueue.take(), it will not return null. Once taken, it will be removed from this queue. In other words, object from that queue definitely is NOT null.
2. After updating GUI, I should manually set it to null so that GC can reclaim its memory quickly.
3. I need to lock that object or set it as atomic.

Thanks.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Edward Chen wrote:
Regarding the double check,
1. I should modify it to resultQueue.take(), it will not return null. Once taken, it will be removed from this queue. In other words, object from that queue definitely is NOT null.
2. After updating GUI, I should manually set it to null so that GC can reclaim its memory quickly.
3. I need to lock that object or set it as atomic.


I agree with 1. That would be good. Just be aware that take() can block. If it is blocked when the app is supposed to shut down, then the app may not shut down properly. You will need some signal that the blocking should end to prevent the threads from waiting forever. I usually use a 'poison pill' - a specific Object that can be identified as the last Object in the queue. When the consumer pulls the poison pill it needs to recognize it and shut down the loop.

For 2 - there is no need to set the value to null. First - it's final you can't reassign it. Second - the object will be available for GC as soon as all its references run out of scope, which for method local variables would be the end of the method. If the variable wasn't final setting it to null wouldn't hurt, it just isn't necessary.

For 3 - Making the reference to Security in the updateFeed or receiveFeed methods does not need to be made Atomic. You may need to synchronize on the Security Object inside the processFeed or updateGUI method. I don't see a need for synchronizing on the Security Object outside those methods, though... My goal is always to keep the synchronized blocks as small as possible while doing its job.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
 
subject: simulation of trading application, thread safety issues ?