java concurrency - synchronized block

[Aleksandar Mitrev]

Hi everyone
I have a thread issue in my code that should not be happening - but is. So I'm trying to make some work around. I will try to explain my problems with simple code as I can - because the code that I'm experiencing the issue is big and complicated so in short the code:

...................
..................
void createAndRunThreads(){
   List<Path> pathList = //read path from DB readPath();
   for(Path p : pathList){
      RunJob rj = new RunJob(p);
      Thred t = new Thread(rj);
      t.start();
   }
}

class RunJob implements Runnable {
   private Path path;
   private ExecuteJob execJob;

   public RunJob(Path path){
      this.path = path;
      this.execJob = new ExecuteJob();
   }

   public void run() {
      execJob.execute(path);
   }
}


class ExecuteJob {

   private static Job curentExecutingJob;

   public void execute(Path path){
      //hear I'm reading the jobs for the specific path from database
      //and I'm getting jobs that are contained in both list e.g. the list created by thread-1 and list created by thread-2 contains the same job jobObject with id=1111 
      List<Job> jobList = getDBJobsFromPath(path);

      for(Job job : jobList) {
         //here the problem is happening when thread-1 e.g. gets to "job" object with id=1111 and after entering if(true) statement it is stop by JVM and thread-2 gets chance to run by the JVM
         //and thread-2 is in a point where "job" object is the same like it was in thread-1 (job.id=1111) and enters the if statement and starts executing - sum initialization, databese changes and job.run
         if(job.getStatus.equals("redye")){
            //do sum initialization 
            //and databese changes 
            job.run();
            job.setStatus("running");
         }    
      }
   }
}

So my problem begins when two thread are trying to execute-read-write changes on the same jobObject at the same time that makes mess in my application
What I need is a way that when a thread is entering "if(job.getStatus.equals("redye")){" with e.g. job object with id=1111 no other thread can enter this code for the same jobObject-1111 until the first thread is finished - leaves the if statement
I also have problems synchronizing it because the "execute" method is call with different instance of ExecuteJob
I can use ExecuteJob.class for synchronization but I will lose the point and benefits of multithreading
I'm wondering if there is something like this that can be implemented for my issue:

class ExecuteJob {
   private static Job curentExecutingJob;

   public void execute(Path path){
      List<Job> jobList = getJobsFromPath(path);

      for(Job job : jobList) {
         curentExecutingJob=job;

         //work around (I don't know if this is possible do) 
         synchronized(curentExecutingJob){
            if(job.getStatus.equals("redye")){
               //do sum initialization 
               //and databese changes 
               job.run();
               job.setStatus("running");
            }   
         }  
      }
   }
}

or maybe

class ExecuteJob {
   private static String curentExecutingJobId;

   public void execute(Path path){
      List<Job> jobList = getJobsFromPath(path);

      for(Job job : jobList) {
         curentExecutingJobId=job.getId().toString();

         //work around (I don't know if this is possible do) 
         synchronized(curentExecutingJob){
            if(job.getStatus.equals("redye")){
               //do sum initialization 
               //and databese changes 
               job.run();
               job.setStatus("running");
            }   
         }  
      }
   }
}

[Steve Luke]

Don't use a static variable to store the thing - that static variable is used by all instances, and so you would have synchronization issues with it. But there is no need for the variable anyway - just use the method-local Job variable:

      for(Job job : jobList) {
         synchronized(job){
            if(job.getStatus.equals("redye")){
               //do sum initialization 
               //and databese changes 
               job.run();
               job.setStatus("running");
            }   
         }  
      }

What I don't know is if the same Job instance is used by all threads, or if getDBJobsFromPath(path) creates new instances each time it is called. If it creates new instances, then we have to see more about the problem, but maybe using database locks would be best.

[Aleksandar Mitrev]

Thanks Steve for your quick reply
So in my case the same record is retrieved from DB-tabale job, but two different instance are been created for example:
Thread-1 gets it from DB and initialize Job job1 = readByIdFromDB("1111")
Thread-2 gets it from DB and initialize Job job2 = readByIdFromDB("1111")
so the expression job1==job2 gives false
but for job1.equals(job2) will return true
From what you are saying and I was able to understand synchronization wont going to work for me in this case. So can you tell me more about that database locks, how can I use it in my problem. I'm using postgres and I'm not rely familiar with this

[Steve Luke]

Here is the Postgres documentation on locking: http://www.postgresql.org/docs/8.2/static/explicit-locking.html You will need to match the level of locking to whe Object you want to lock, not sure if it is Row or Table. What I would think you would do is provide each Job with a reference to some Database Access Object which knows which Table/Row to lock for that Job.

public class JobLock
{
    private final String id;
    private volatile Connection connForLock;
    
    public JobLock(String jobId)
    {
        this.id;
    }

    public void lock()
    {
        connForLock = //get connection
        /* get a row lock */
        /* run query SELECT FOR UPDATE WHERE jobid = id */
    }

    public void unlock()
    {
        /* finish transaction, releasing lock */
        conn.commit();
    }
}

public class Job
{
    private final volatile JobLock lock;

    public Job(/* ... */)
    {
        //...
        this.lock = new new JobLock(this.id);
    }

    public JobLock getLock()
    {
        return this.lock;
    }
}

Then you could use the code like:

      for(Job job : jobList) {
         job.getLock().lock();
         try
         {
            if(job.getStatus.equals("redye")){
               //do sum initialization 
               //and databese changes 
               job.run();
               job.setStatus("running");
            }   
         } 
         finally
         {
             job.getLock().unlock();
         }
      }

All the other DB calls would use the same connection the lock creates, and the unlock would commit (or rollback) the transaction. But to be honest my JDBC knowledge is getting rusty and I have never used PostgreSQL, so use with care.