Search code examples
javamultithreadingconcurrencysynchronizedsynchronize

Problem in multi-threaded Java application - synchronized method not working as expected


I am coding a program similar to producer-consumer in java (but with the consumer only portion, with no producer thread). It looks like the critical region is being executed at the same time by multiple threads despite the fact that I am calling the code of a synchronized method.

Here is the code:

Class Main.java:

package principal;

public class Main {

/**
 * @param args
 */
public static void main(String[] args) {
    final int tamanho_buffer=10;
    final int quantidade_threads=10;

    Pedido buffer[] = new Pedido[tamanho_buffer];
    Consumidor consumidor[] = new Consumidor[quantidade_threads];

    for (int i=0;i<tamanho_buffer;i++) {
        buffer[i]=new Pedido();
    }

    for (int i=0;i<quantidade_threads;i++) {
        consumidor[i]=new Consumidor();
    }

    for (int i=0;i<tamanho_buffer;i++) {
        int identificador[]=new int[Pedido.getTamanho_identificador()];
        identificador[0]=i;
        buffer[i].setIdentificador(identificador);
        buffer[i].setTexto("pacote de dados");
    }

    Consumidor.setBuffer(buffer);
    Consumidor.setTamanho_buffer(tamanho_buffer);

    for (int i=0;i<quantidade_threads;i++) {
        consumidor[i].start();
    }

    for (int i=0;i<quantidade_threads;i++) {
        try {
            consumidor[i].join();
        }catch(InterruptedException e ){
            System.out.println("InterruptedException lancada");
        }
    }

    System.out.println("Processamento encerrado.");

}

}

Class Pedido.java:

package principal;

    public class Pedido {
private int identificador[];
private String texto;
static int tamanho_identificador=10;
int ti=tamanho_identificador;

public Pedido() {
    this.identificador= new int[this.ti];
}

public static int getTamanho_identificador() {
    return tamanho_identificador;
}

public int[] getIdentificador() {
    return identificador;
}
public void setIdentificador(int[] identificador) {
    this.identificador = identificador;
}
public String getTexto() {
    return texto;
}
public void setTexto(String texto) {
    this.texto = texto;
}

}

Class Consumidor.java

package principal;

import java.util.Date;
import java.text.DateFormat;
import java.text.SimpleDateFormat;

public class Consumidor extends Thread {

private static Pedido buffer[];
private static int tamanho_buffer;
private static int posicao=0;

public static void setTamanho_buffer(int tamanhoBuffer) {
    tamanho_buffer = tamanhoBuffer;
}

public static void setBuffer(Pedido[] buffer) {
    Consumidor.buffer = buffer;
}

public void run() {
    DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");
    while (posicao < Consumidor.tamanho_buffer ) {
   //           int identificador;
   //           identificador=buffer[posicao].getIdentificador()[0];;

        Date datainicio = new Date();
        String inicio=dateFormat.format(datainicio);

        try {
            Consumidor.sleep(1000);
        } catch(InterruptedException e) {
            System.out.println("InterruptedException lancada");
        }

        Date datafim = new Date();
        String fim=dateFormat.format(datafim);

        consomebuffer(inicio,fim);

    }
}

synchronized void consomebuffer(String inicio, String fim) {
    if (posicao < Consumidor.tamanho_buffer ) {
        int identificador;
        identificador=buffer[posicao].getIdentificador()[0];
        System.out.println("Thread: "+Thread.currentThread()+" Pedido: "+identificador+" Inicio: "+inicio+" Fim: "+fim+" posicao "+posicao);
        posicao++;
    }
}
}

The method consomebuffer is synchronized but it looks like the variable posicao (portuguese name for position) is being accessed by the other threads at the same time, that should not happen, coz it is a synchronized method. The program output is something like this:

Thread: Thread[Thread-7,5,main] Pedido: 0 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 0

Thread: Thread[Thread-6,5,main] Pedido: 0 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 0

Thread: Thread[Thread-2,5,main] Pedido: 0 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 0

Thread: Thread[Thread-9,5,main] Pedido: 0 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 0

Thread: Thread[Thread-3,5,main] Pedido: 4 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 4

Thread: Thread[Thread-5,5,main] Pedido: 5 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 5

Thread: Thread[Thread-0,5,main] Pedido: 0 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 5

Thread: Thread[Thread-8,5,main] Pedido: 0 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 5

Thread: Thread[Thread-4,5,main] Pedido: 5 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 5

Thread: Thread[Thread-1,5,main] Pedido: 0 Inicio: 2011/09/24 21:14:18 Fim: 2011/09/24 21:14:19 posicao 0

Processamento encerrado.

Realize that the position value is appearing repeated between different threads. In the output posicao should have a different value in each thread that calls the synchronized method.


Solution

  • Each Thread instance is synchronising on itself. To make all of the threads mutually exclusive, they have to synchronise on a common object.

    That is,

    public synchronized method(int parameter)
    {
        //do some stuff
    }
    

    is shorthand for

    public method(int parameter)
    {
        synchronized (this)
        {
            //do some stuff
        }
    }
    

    If you want a bunch of Threads to synchronise with each other, you need to make available to them a common object to synchronize on.

    For example, you could add in the constructor of Consumidor.java

    public Consumidator(Object monitor)
    {
         myMonitor = monitor
    }
    

    then in run have

    void consomebuffer(String inicio, String fim) {
      synchronized (myMonitor)
      {
        if (posicao < Consumidor.tamanho_buffer ) {
            int identificador;
            identificador=buffer[posicao].getIdentificador()[0];
            System.out.println("Thread: "+Thread.currentThread()+" Pedido: "+identificador+" Inicio: "+inicio+" Fim: "+fim+" posicao "+posicao);
            posicao++;
        }
      }
    }
    

    Then when you create your array of Consumidadors, pass them a shared object to synchronize on.