Multithreading headache. Java. Return values

Hi guys I have the following.

class a extends Thread    
{
    public synchronized BigInteger getUniqueID()
    {
        BigInteger aUniqueID = new BigInteger(getUniqueKeyFromDatabase);
        return aUniqueID;
    }   
}

class b extends a
{
    public run()
    {
        BigInteger uniquieID = getUniqueID();       
        // store UniqueID in another database table with other stuff
     }
}

And what I'm getting is duplicate unique id stored in the database table. I'm assuming because uniqieID is being changed in this multi threaded environment.

I'm obviously going horribly horribly wrong somewhere, I'm guessing I shouldn't be returning the value in this way. Or sho开发者_Python百科uld be defining uniqueID as new BigInteger based on the response from the getUniqueID method.

Any help would be greatly appreciated, as my fragile mind has been warped right now!

Cheers

Alan

---

BigInteger is an (from the JavaDocs)

Immutable arbitrary-precision integer

So that rules out anyone mutating the BigInteger object. I'd look into getUniqueKeyKeyFromDatabase

---

You getUniqueKeyFromDatabase() has to be a method which will not return the same value twice. Everything else doesn't matter.

Each thread has it own copy of local variables are they are not shared.

BTW: don't extend Thread, its bad practice which often leads to confusion.

---

Your problem is because you're not really synchronizing anything. The getUniqueID() method in class A is synchronized on its own implicit monitor. But that means each time you create a new thread, you're synchronizing each one on itself. Does that make sense ?

You need to synchronize on some shared variable. A quick fix to illustrate the point (but really don't use this in practice) is: In the example below all your threads are synchronizing on the same object ( a shared static ).

class A extends Thread {

    static Object shared = new Object();

    public BigInteger getUniqueID()
    {
       synchronize (shared) {
         BigInteger aUniqueID = new BigInteger(getUniqueKeyFromDatabase);
         return aUniqueID;
       }
    }

}

---

Chances are, that the synchronized modifier for getUniqueID() is pointless, as you don't modify any state there. It does not protect the getUniqueKeyFromDatabase() either, because it synchronizes on the instance. This means that every Thread runs without synchronizing with the others.

You could try if

public BigInteger getUniqueID() {
   synchronized (a.class) {
      BigInteger aUniqueID = new BigInteger(getUniqueKeyFromDatabase);
      return aUniqueID;
   } 
}

Works better for you. If it does, you should think about your database design (or whatever happens in getUniqueKeyFromDatabase). Synchonization should really be done by the database, not in client code.

---

You must have a problem with the method which returns the unique id. To assure uniqueness of your ids for each object use something like below, for example in you class a.

PS: Class names should start from capital letters. Also as suggested by @Peter Lawrey implement Runnable instead of extending a Thread.

    private static int nextId = 0;
    protected int id;

    public a(){
       this.id = getNextId();
    }
    private static int getNextId(){
        return nextId++;
    }