CODEWITHSUNDEEP
java
ASD
ASD

Reputation: 735

Why I am getting deadlock

I cannot understand why I am getting deadlock in this simple sample.What is wrong with it?

public static void main(String[] args) {
        Object data = null;
        new Thread(new Producer(data)).start();
        new Thread(new Consumer(data)).start();
    }
}

class Producer implements Runnable {
    private Object data;

    public Producer(Object data) {
        this.data = data;
    }

    @Override
    public void run() {
        while (true) {
            while (data != null) {}
            data = new Object();
            System.out.println("put");
        }
    }
}

class Consumer implements Runnable {
    private Object data;

    public Consumer(Object data) {
        this.data = data;
    }

    @Override
    public void run() {
        while (true) {
            while (data == null) {  }
            data = null;
            System.out.println("get");
        }
    }

Upvotes: 0

Views: 100

Answers (5)

Sandro
Sandro

Reputation: 1276

I think this should do what you intend (it is still bad code):

public class Example {
    public static void main(String[] args) {
        Consumer consumer = new Consumer();
        new Thread(new Producer(consumer)).start();
        new Thread(consumer).start();
    }
}

class Producer implements Runnable {
    private final Consumer consumer;

    public Producer(Consumer consumer) {
        this.consumer = consumer;
    }

    @Override
    public void run() {
        while (true) {
            while (consumer.data != null) {}
            consumer.data = new Object();
            System.out.println("put");
            try {
                Thread.sleep(5);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

class Consumer implements Runnable {
    public volatile Object data;

    @Override
    public void run() {
        while (true) {
            while (data == null) {}
            data = null;
            System.out.println("get");
            try {
                Thread.sleep(5);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

I think you should focus on the basics of Java, before you go for advanced topics such as parallel programming as the main error in you example (separate data fields) is very basic.

Upvotes: 1

trutheality
trutheality

Reputation: 23455

When your produced "produces", all it does is points its own data reference to the new object, and the consumer has no way of knowing what happened. What you can do instead is make another class

class Data {
    private Object data = null;
    synchronized void set( Object data ){ this.data = data; }
    synchronized Object get(){ return data; }
}

Then in your main do

Data data = new Data();

Pass the 'Data' object to the consumer and producer, and use the get/set methods instead of assignment.

That way, both consumer and producer will be pointing to the same Data object and when the producer produces or the consumer consumes, they will be changing the reference in the Data object, which they are sharing.

Upvotes: 1

Joachim Isaksson
Joachim Isaksson

Reputation: 180877

The consumer and producer have separate data fields, so the consumer will never get any data to consume.

Also, spinlocking a consumer/producer on a field isn't generally a good idea, you're much better off using mutexes or semaphores to signal the availability of data / the possibility to publish. If this is more than a test in the search of knowledge, you should really read up on how to use those two.

Upvotes: 2

Affe
Affe

Reputation: 47954

There are two problems.

1: You have two separate Runnables that each have their own private internal member named data. Changes made to one aren't visible to the other. If you want to pass data between two threads, you need to store it in a common place where they both access it. You also need to either synchronize around the accesses or make the reference volatile.

2: Your checks seem to be inverted. You probably want to null it when it's not null, and create one when it is null? Its tough to tell what you want it to actually do there! :)

public static volatile Object data;

public static void main(String[] args) {
        data = null;
        new Thread(new Producer(data)).start();
        new Thread(new Consumer(data)).start();
    }
}

class Producer implements Runnable {

    public Producer(Object data) {
        this.data = data;
    }

    @Override
    public void run() {
        while (true) {
            while (data == null) {}
            data = new Object();
            System.out.println("put");
        }
    }
}

class Consumer implements Runnable {

    public Consumer(Object data) {
        this.data = data;
    }

    @Override
    public void run() {
        while (true) {
            while (data != null) {  }
            data = null;
            System.out.println("get");
        }
    }

(Also this isn't really an example classically what we'd define as a deadlock, where two threads can't proceed because they both want locks the other has. There are no locks here. This is an example of two infinite loops that just don't do anything.)

Upvotes: 3

SLaks
SLaks

Reputation: 887245

Each instance has its own data field.
The consumer never sees the producer's changes.

Upvotes: 2

Related Questions