It's not a secret anymore!
The moose likes Threads and Synchronization and the fly likes Issue with multithreading Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Issue with multithreading" Watch "Issue with multithreading" New topic

Issue with multithreading

parnab chanda

Joined: Jun 11, 2013
Posts: 1
I have an integer ArrayList of 50 numbers where each number is an ID . From the main program i want to read the 50 numbers in a loop and and for each id i want to create a thread.There a global ArrayList of String . Each thread should insert to the String ArrayList its ID . I paste the code below .So at the end the String ArrayList should also be of size 50 with all the IDs inserted atmost once . However i am finding that not all 50 are getting processed .At times 42,at times 43 or 48 and these numbers are varying inconsistently.
Can someone please help where i am going wrong .

Inesh Hettiarachchi

Joined: Feb 07, 2004
Posts: 25
Dear Chanda,
well come to code ranch.
Synchronized code should be in a common place where multiple threads can access but you have synchronized the addElement method which is in the thread it self and it will be only accessed by the thread itself it is not visible to other threads. This will not prevent from other threads from making a data race on the myList they will store data in the same list position, that's why its actually finishing with lesser than number 49 and 50 elements in the array list.

you can do two things
change myList to a collection from the concurrent API and remove your synchronized key word from the addElement , any way its not synchronizing any thing.
move the addElement to the another class just add it to the ThreadPoolTest.class as a static method or create a singleton class and the method synchronized.
but i prefer the first one.

hope these links will help you.

Chan Ag

Joined: Sep 06, 2012
Posts: 1089
In my opinion, the issue here is that although your addElement method is synchronized, myList is a shared List among all the 50 threads. The addElement method, in effect, is doing only this-

Making sure no other thread on the same Processor object is able to invoke addElement.

But in your case they are all new Processor(). So I'm not sure if you'd really want to add the overhead of synchronizing at the Processor instance level.

So I don't think your synchronization at the method level is achieving anything.

I guess this is not what you want. You want to protect your list which is a shared resource. So you might instead want to change your code as :-

Let us know if it worked -
I agree. Here's the link:
subject: Issue with multithreading
It's not a secret anymore!