Exception thrown when two threads manipulate a list, synchronized via semaphores

Question

Trying to process a list of Order objects with two different threads, but running into the following exception:

    Exception in thread "Thread 1" java.lang.IndexOutOfBoundsException: Index: 4, Size: 4
        at java.util.ArrayList.rangeCheck(Unknown Source)
        at java.util.ArrayList.get(Unknown Source)
        at TaskThread.run(PrintOrder_Priority.java:193)
        at java.lang.Thread.run(Unknown Source)
    Thread 1:Processing Order234,7
    Thread 2:Processing Order235,6
    Thread 1:Processing Order237,5
    Thread 2:Processing Order236,4

I have just omitted the Order class details, which contains fields such as id and priority. I have sorted the list of orders based on their priority, and want to process the orders.

class TaskThread implements Runnable 
{
    List<Order> orderList;
    static volatile int i=0;

    Semaphore sem1;
    Semaphore sem2;

    TaskThread(List<Order> orderList,Semaphore sem1,Semaphore sem2)
    {
        this.orderList=orderList;
        this.sem1=sem1;
        this.sem2=sem2;
    }

    @Override
    public void run()
    {
        for(;i<orderList.size();)
        {
            try
            {
                sem1.acquire();
                orderList.get(i).ProcessOrder();
                i++;
                sem2.release();
            } 
            catch (InterruptedException e) 
            {
                e.printStackTrace();
            }
        }
    }
}

Contents of the main method:

 Semaphore sem1 = new Semaphore(1); 
 Semaphore sem2 = new Semaphore(0);
 Thread T1 = new Thread(new TaskThread(aList,sem1,sem2));
 Thread T2 = new Thread(new TaskThread(aList,sem2,sem1));
 T1.setName("Thread 1");
 T1.start();
 T2.setName("Thread 2");
 T2.start();

Why don't you just use a `BlockingQueue` for your task list? That would get rid of your indexing issue. — daniu, Jun 21 '19 at 21:02
That Ok @daniu , but my problem here is how the thread is getting passed the orderlist size() condition and giving me the IndexOutOfBoundsException: Index: 4, Size: 4 (where as I am using i as static volatile variable) , current outcome is not expected in this case — Nitin Goyal, Jun 22 '19 at 08:01

HPCS · Accepted Answer · 2019-06-24T08:46:19.030

Your problem is this, the size condition is check before you acquire the semamphore and when you acquire the semaphore the value of i is different than the value you check in the condition:

Thread1:                                 Thread2:
condition check: i ==0; OK;                condition check: i ==0; OK; 
acquire sem1:                              waiting form sem2
process i =0;                              waiting form sem2  
increase i to 1;                           waiting form sem2
release sem2;                              you acquire sem2 but  i ==1; 
condition check: i ==1; OK;                 process i =1;
waiting form sem1                          increase i to 2;  
waiting form sem1                          release sem1; 
acquire sem1 but i ==2;                     condition check: i ==2; OK;    
process i =2;                              waiting form sem2    
increase i to 3;                           waiting form sem2
release sem2;                              you acquire sem2 but  i ==3; 
condition check: i ==3; OK;                 process i =3;
waiting form sem1                          increase i to 4;  
waiting form sem1                          release sem1; 
acquire sem1 but i==4!!!!!!
you call ->> orderList.get(i) -> exception thrown

The easiest way how to fix it if you want to use your solution is to add check also after you acquire the monitor

sem1.acquire();
if(i >= orderList.size()){
break;
}

Thanks @HPSC ,Now I realized My mistake , Do you any other elegant solution in mind ? — Nitin Goyal, Jun 24 '19 at 14:18

Jazzepi · Answer 2 · 2019-06-22T21:03:49.890

0

https://www.baeldung.com/java-even-odd-numbers-with-2-threads

Meant to update this and forgot. This example seems to show two threads alternating on semaphores like yours was. They aren't sharing a variable, but it's pretty close.

edited Jun 22 '19 at 21:03

answered Jun 21 '19 at 15:55

Jazzepi

5,259
11
55
81

Thanks for the answer , but this solution is also giving the same error ,moreover it will not solve my problem statement , I am acquiring a lock inside for look , even after releasing , I cant ensure that my order is getting processed alternately by T1, T2. – Nitin Goyal Jun 21 '19 at 20:27
@NitinGoyal Ohh sorry I missed that part about your problem statement. I think I found an example you can crib from. – Jazzepi Jun 21 '19 at 20:35
could you please share the Example. – Nitin Goyal Jun 22 '19 at 08:02
@NitinGoyal Updated sorry about that. – Jazzepi Jun 22 '19 at 21:04
@ Jazzepi - Thanks – Nitin Goyal Jun 24 '19 at 14:24

score 0 · Answer 3 · answered Jun 21 '19 at 20:43

0

Might be the volatile keyword. According to this article, volatile gives a "happens-before" guarantee, but not more. Maybe try an AtomicLong

answered Jun 21 '19 at 20:43

KoW

784
5
12

:- This is not helping ,Getting the same Exception. – Nitin Goyal Jun 21 '19 at 20:52
Ok, checked on Semaphore: the argument in the constructor is the number of available permits. Maybe have to initialize twice with Semaphore(1) – KoW Jun 21 '19 at 21:00

Exception thrown when two threads manipulate a list, synchronized via semaphores

3 Answers3