Aiden
Aiden

Reputation: 460

Static method with mutable parameter thread safe?

I have a method

public static Person updatePersonId (Person person)
{
    // If the ID of the person reaches the maximum ID in our predefined range in configuration then reset the ID from the start otherwise it will cross the range we defined.
    if (person.getNewID ().longValue () == person.getLastPossibleID ().longValue ())
    {
        person.setNewID (person.getFirstID ());
    }

    // add 1 to Old ID to get new ID
    person.setNewID (person.getNewID + 1);

    return person;
}

I use hibernate to update it into database.

As you can see, I can't have more than one person in database with same ID (as we always add 1 to its previous ID).

Now the problem is, when I run my application concurrently, i.e. two transactions at same time, the ID assigned to person become duplicate sometimes e.g.

Database row

PersonID        PersonName

  1                  Bob
  2                  Robert
  2                  Daniel

Is the method i created, not thread safe ? Shall I add Synchronized keyword ?

Upvotes: 0

Views: 206

Answers (1)

Panther
Panther

Reputation: 3339

Yes, this method is not threadsafe. As, two threads may reach to this line at sametime :-

person.setNewID (bnkPrb.getNewID + 1);

However syncronizing whole method will slow downthe codes, so take lock on only this line :-

public static Person updatePersonId (Person person) { // If the ID of the person reaches the maximum ID in our predefined range in configuration then reset the ID from the start otherwise it will cross the range we defined

if (person.getNewID ().longValue () == person.getLastPossibleID ().longValue ())
{
    person.setNewID (person.getFirstID ());
}

// add 1 to Old ID to get new ID
syuncronized(this){
person.setNewID (bnkPrb.getNewID + 1);
}


return person;
} 

However , best approach to have automincremented column or sequence at database side.

Upvotes: 1

Related Questions