Java Locks synchronize methods baking transfer

Question

Hello I have a Java application that takes an input number of operations to perform and it run different threads for each operation:

//create operations to execute
Thread t[] = new Thread [n_operations];

//we create a Bank with N accounts
Bank mybank = new Bank(N);

//execute a separate thread per operation
for (int i = 0; i < n_operations; i++) {
    int id = i;
    Operation o = new Operation(mybank, id);
    t[i]= new Thread (o);
    t[i].start();
}
for (int i=0;i<N;i++){
    try{
        t[i].join();
        }catch(Exception e){;}
}

Now I need to perform concurrent transfer on the accounts, where the Bank class is defined like this:

public class Bank {

    private static        Account[] accounts;
    final  int      MAX_balance  = 100000;
    int         MAX_accounts = 0;

    /* Create accounts of a bank */
    public Bank (int N) {

        accounts = new Account[N];
        MAX_accounts = N;

        for (int i = 0; i < N; i++)

            accounts[i] = new Account (i, 1000);

    }
    public int getN(){
        return MAX_accounts;
    }

    public synchronized int transfer(int from, int to, int amount) {


            synchronized (accounts[from]){
          synchronized (accounts[to]){

          if (accounts[from].balance () < amount) {

              try{
              System.out.println("Error during transfer: Not enough Money");
              }
              catch(Exception err){ 
                  return 1;
              }              
         }

         accounts[from].sub(amount);
         accounts[to].add(amount);
        }
    }

    return 0;
    }
}

When the program performs the operations:

public class Operation implements Runnable {

    private Bank      b;
    int id;
    Random r;
    private final int  MAX_TRANSFERENCIAS = 1000;

        public Operation (Bank b, int id) {

            this.b = b;
            this.id = id;

    }

    public int syncronize(){
        return 1;       
    }

    public void run () { 


        r = new Random();
        if(b == null)
              throw new RuntimeException("b is null!");
            if(r == null)
              throw new RuntimeException("r is null!"); 
        int max = b.getN();
        //depend if there is a conflict or not

            b.transfer (id,r.nextInt(max),r.nextInt(100));

    }
}

I get a series of errors like this message:

        at Bank.transfer(Bank.java:28)         /* which is "synchronized (accounts[from]){" */

        at Operation.run(Operation.java:33)    /* which is "b.transfer 
(id,r.nextInt(max),r.nextInt(100));" */

        at java.lang.Thread.run(Unknown Source)
    java.lang.ArrayIndexOutOfBoundsException: 4714

Do you think the Synchronization is ok?

Any suggestions? Many thanks

UPDATE (I can't answer myself)

There is a concept error in the main loop (for i..to n_operations), the function is passing "int id = i;" as parameter for the source_account, while the n_operation number is bigger than the max value of the array, so the compiler reasonably says: ArrayIndexOutOfBoundsException.

As final contribution I would ask you to kindly check if the Synchronization is done correctly, as I am not an expert in multithreading. Many thanks again, and sorry for badly formulate the question this morning....

Look on line 27 of your `Operation.java` class and debug which variable is `null` at the time. We can't see it, so we don't know what's the problem. — Kiril, Mar 06 '12 at 15:45
the line 27 is "b.transfer (id,r.nextInt(max),r.nextInt(100));" — nuvio, Mar 06 '12 at 17:18
So most likely `b` or `r` is null. `id` or `max` could also cause a NPE if either is an `Integer` that is `null` and gets auto-boxed. — Gray, Mar 06 '12 at 17:31
r was null as I deleted the initialization of r, now it should be fine, but the "Array out of bound" error persists.... — nuvio, Mar 06 '12 at 17:35

Gray · Accepted Answer · 2012-03-06T17:33:12.020

3

Edit:

Now that we know that the following line is the source of the NPE:

b.transfer (id,r.nextInt(max),r.nextInt(100));

So most likely b or r is null. You should put a break point there and debug into it to see if they are. You could also use assert or logging to display the values. Remember also that id or max could also cause a NPE if either is an Integer that is null and gets auto-boxed.

This wouldn't cause your NPE but be careful that n_operations may not be == 100? You are starting n_operations threads but joining with 100 of them:

for (int i=0;i<100;i++){
    try {
        t[i].join();
    } catch(Exception e){;}
}

I always use the length of the array in these cases so you don't have a mismatch between what was allocated:

for (int i = 0; i < t.length; i++) {

Also, at the very least you should always log or print your exceptions. Catching and dropping exceptions often means you are hiding important debugging information.

    } catch(Exception e){ e.printStackTrace(); }

edited Mar 06 '12 at 17:33

answered Mar 06 '12 at 15:44

Gray

115,027
24
293
354

That would be the wrong number to join on, but that would throw an index out of range exception because he doesn't have that many threads in the array. – Kiril Mar 06 '12 at 15:46
Good point @Lirik. I didn't see the initialization at the top of the code. Thanks. – Gray Mar 06 '12 at 15:50
Actually, I hadn't even noticed the initialization, so the only thing that can happen now is that he can only get an index out of range exception when the number of threads is less than 100. – Kiril Mar 06 '12 at 15:56
Allright thank, that 100 actually was an error from a previous test, but I still have the problem. I will update me question now with the new code... – nuvio Mar 06 '12 at 17:27

score 1 · Answer 2 · answered Mar 06 '12 at 15:48

1

One of the variables you are using in run() is null, but which one? Try adding the following to the beginning of Operation.run():

if(b == null)
  throw new RuntimeException("b is null!");
if(r == null)
  throw new RuntimeException("r is null!");

I presume that the lines you showed from run() include line 27. If not, please post the full source code for run().

answered Mar 06 '12 at 15:48

Alex D

29,755
7
80
126

Note this will not fix your problem; it is just intended to help you find out which of your instance variables is null, so you can do something about it. – Alex D Mar 06 '12 at 15:48

Java Locks synchronize methods baking transfer

2 Answers2