CODEWITHSUNDEEP
javamultithreading
Serhii
Serhii

Reputation: 101

How to synchronize methods in java?

i have some small problem. I hava some class Processor and Process. Process class have only name and value Processor must calculate this value and display it. In turn, Processor implement Runnable interface But i have a trouble. When I set process to my Processor, it begin to calculate value, but i want to start another thread with different value of process, and this value set to first thread too. Here is code

    package ua.edu.lp.controller;

import java.util.Random;

import ua.edu.lp.controller.processors.FirstProcessor;
import ua.edu.lp.controller.processors.MyBaseProcessor;
import ua.edu.lp.model.MyProcess;
import ua.edu.lp.model.Queue;

public class Computer {

    private Queue<MyProcess> queue;

    public void run() {
        MyBaseProcessor myBaseProcessor = new FirstProcessor();
        generateQueue();

        for (int i = 0; i < queue.getQueue().size(); i++) {
            MyProcess process = queue.get(i);
            myBaseProcessor.setProcess(process);
            Thread thread = new Thread(myBaseProcessor);
             thread.start();
        }
    }

    private void generateQueue() {
        queue = new Queue<MyProcess>();
        for (int i = 0; i < 10; i++) {
            String name = "Process[" + (i + 1) + "]";
            MyProcess myProcess = new MyProcess(name,
                    new Random().nextInt(10) + 1);
            queue.add(myProcess);
            System.out.println(myProcess.getName() + "\t"
                    + myProcess.getValue());
        }
    }
}

BaseProcessor

package ua.edu.lp.controller.processors;

import ua.edu.lp.model.MyProcess;

public abstract class MyBaseProcessor implements Runnable {

    protected MyProcess myProcess;

    @Override
    public void run() {
        synchronized (myProcess) {
            calculateValue();
        }
    }

    protected abstract void calculateValue();

    public synchronized MyProcess getProcess() {
        return myProcess;
    }

    public synchronized void setProcess(MyProcess myProcess) {
        synchronized (myProcess) {
            this.myProcess = myProcess;
        }

    }

}

And FirstProcessor

package ua.edu.lp.controller.processors;

public class FirstProcessor extends MyBaseProcessor {

    @Override
    protected synchronized void calculateValue() {
        System.out.println("First processor is working...");
        int value = this.myProcess.getValue();
        int result = (int) Math.pow(value, 2);
        System.out.println("Pow of " + value + " = " + result);
        System.out.println("First processor is free");
    }

}

All project you can download from http://www.sendspace.com/file/5rlmkx

Upvotes: 2

Views: 836

Answers (3)

Peter Lawrey
Peter Lawrey

Reputation: 533472

All your task are independant so you shouldn't need synchronized anywhere. However YOu are sharing a BaseProcessor object which while synchronized, is not synchronized for the life of the processing.

You only need synchronized to process shared data where one thread can modify that data after the threads have started.

BTW: I would use an ExecutorService as this wraps the Queues and Thread pools in one class.

I assume your Queue is thread safe because not all Queues are in Java. It could be a problem if it wasn't.

This program does the same thing, however its simpelr and thread safe.

ExecutorService service = Executors.newSingleThreadExecutor();
Random random = new Random();
for (int i = 0; i < 10; i++) {
    final String name = "Process[" + (i + 1) + "]";
    final int value = random.nextInt(10) + 1;
    service.execute(new Runnable() {
        @Override
        public void run() {
            System.out.println("First processor is working...");
            int result = (int) Math.pow(value, 2);
            System.out.println("Pow of " + value + " = " + result);
            System.out.println("First processor is free");
        }
    });
    System.out.println(name + "\t" + value);
}
service.shutdown();

Upvotes: 1

miwoe
miwoe

Reputation: 797

synchronized (myProcess)

will only set a MUTEX for the myProcess object. Synchronization between different MyBaseProcessor objects will only work, if both objects point to the the same myProcess object.

I don't know what the concrete requirements are, but it looks correct for me=> Not more than one processor whould work on a single process?

A bit confusing for me is why you are also synchronizing in the setterMethod setProcess(..) with the target field. This does not have any effect.

Maybe you didn't understand that the synchronization will be done with the object and not the type?

BR - Michael

Upvotes: 1

se.solovyev
se.solovyev

Reputation: 1901

Make Processor stateless - remove field process and do something like this:

final Processor processor = new Processor();

processor.process(new Process(...));

public void process( final Process process ) {
    new Thread(new Runnable() {
        //do somethig with process
        }).run();
}

NOTE: Processor must do only common task - to run process, and process implement business logic and only process aware of calculations you've done.

Upvotes: 1

Related Questions