0

The application I'm maintaining (passed through many coders) has the Producer-Consumer problematic implemented using wait/notify mechanism.

Consumer waits for the message on the "server" side of the application and then forwards the message on the "client" side towards the LDAP server.

Problem is when multiple connections are established/terminated. The Producer threads just keep multiplicating and never get terminated when they should.

When a connection is terminated both the Producer/Consumer threads should be terminated as well. With a high number of established/terminated connections, memory usage gets monstrous.

The code:

class Producer extends Thread {
    public void run() {
        long previous = 0;
        long last = 0;
        long sleeptime = 1;

        while (alive) {
            try{
                last = System.currentTimeMillis();

                byte[] aux;
                if ((aux = cliente.readmessage()) != null){

                    sleeptime = 1;
                    previous = last;

                    synchronized (list) {
                        while (list.size() == MAX)
                            try {
                                list.wait();
                            } catch (InterruptedException ex) {
                            }
                        list.addFirst(new Messagetimestamped(aux, System
                                .currentTimeMillis()));
                        list.notifyAll();
                    }
                }
                else{
                    if (last-previous > 1000)
                        sleeptime = 1000;
                    else
                        sleeptime = 1;
                    sleep(sleeptime);
                }
            }
            catch (Exception e){
                if (lives()){
                    System.out.println("++++++++++++++++++ Basic Process - Producer");
                    kill();
                    nf.notify(false, processnumber);
                }
                return;
            }
        }
    }
}


class Consumer extends Thread{

    public void run() {
        while (alive) {
            byte[] message = null;
            Messagetimestamped  mt;
            synchronized(list) {
                while (list.size() == 0) {
                    try {
                        list.wait(); //HANGS HERE!
                        if (!alive) return;
                        sleep(1);
                    }
                    catch (InterruptedException ex) {}
                }
                mt = list.removeLast();
                list.notifyAll();
            }
            message = mt.mensaje;

            try{
                long timewaited = System.currentTimeMillis()-mt.timestamp;

                if (timewaited < SLEEPTIME)
                    sleep (SLEEPTIME-timewaited);

                if ( s.isClosed() || s.isOutputShutdown() ){
                    System.out.println("++++++++++++++++++++ Basic Process - Consumer - Connection closed!(HLR)");
                    kill();
                    nf.notify(false, processnumber);
                }
                else {
                    br.write(message);
                    br.flush();
                }
            } catch(SocketException e){
                return;
            } catch (Exception e){
                e.printStackTrace();
            }
        }
    }
}

Basically after alive is set to false Producer gets actually terminated. Consumer doesn't. It just stays hanging on the list.wait() line. Obviously the list.notify() (or list.notifyAll()?) from Producer isn't delivered after it's terminated so Consumer never gets to check the alive boolean.

How to solve this using as little modifications as possible?

Thanks.

vobelic
  • 51
  • 1
  • 5

1 Answers1

3

I would just use an ExecutorService which will wrap up the queue, manage your thread(s) and handle shutdown for you. Almost all your code will go away if you do.

But to answer your question, I suggest sending a poison pill. A special object which the consumer will shutdown when it receives it.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130