1

My threads are getting mixed up. The result of sendResult() and receiveResponse() should both have different responses (I respond with JSON in a servlet). However they both reply with the response of sendResult().

Can someone explain why this is, and how to solve this?

class Authenticate {
    String t2RequestId = null;
    String finalUserInput = null;

    public synchronized String sendAuthentication(String deviceId, String requestId, String apiKey) {
        // Send notification
        GCM gcmClass = new GCM();
        gcmClass.authenticateRequest(deviceId, requestId, apiKey);

        while(!t2RequestId.equals(requestId)) {
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
        return finalUserInput;
    }

    public synchronized void receiveAuthentication(String userInput, String requestId) {
        finalUserInput = userInput;
        t2RequestId = requestId;
        notifyAll();
    }
}

class T1 implements Runnable {
    Authenticate m;
    private final String deviceId;
    private final String requestId;
    private final String apiKey;
    String result;
    public T1(Authenticate m1, String deviceId, String requestId, String apiKey) {
        this.m = m1;
        this.deviceId = deviceId;
        this.requestId = requestId;
        this.apiKey = apiKey;
        Thread t1 = new Thread(this, requestId);
        t1.start();

        // Wait for thread to finish before sending response
        try {
            t1.join();
        } catch(InterruptedException e) {
            e.printStackTrace();
        }
    }

    public void run() {
       result = m.sendAuthentication(deviceId, requestId, apiKey);
    }
    public String getResult() {
        return result;
    }
}

class T2 implements Runnable {
    Authenticate m;
    private final String requestId;
    private final String userInput;
    public T2(Authenticate m2, String requestId, String userInput) {
        this.m = m2;
        this.requestId = requestId;
        this.userInput = userInput;
        Thread t2 = new Thread(this, "t2" + requestId);
        t2.start();
    }

    public void run() {
        m.receiveAuthentication(userInput, requestId);
    }
}
public class AuthenticationHandler {
    final static Authenticate m = new Authenticate();

    public static String sendRequest(String deviceId, String requestId, String apiKey) {
        T1 runnable = new T1(m, deviceId, requestId, apiKey);
        String result = runnable.getResult();
        return result;
    }
    public static void receiveResponse(String requestId, String userInput) {
        new T2(m, requestId, userInput);
    }
}
Floegipoky
  • 3,087
  • 1
  • 31
  • 47
  • There ar some cases that i didn't not understand. T1 initilized and main thread waits for the T1 thread. But T1 thread waits for response so T2 can not run. Where do you call sendRequest and receiveResponse? – Gurkan İlleez Oct 20 '14 at 20:14
  • Are you sure you got response from servlet? Also notify is not a good solution because it notifies a thread randomly. For example you have 30 thread waiting and you call a notify then you choose a thread randomly – Gurkan İlleez Oct 20 '14 at 20:15
  • Also your classes held states that can be a big problem. – Gurkan İlleez Oct 20 '14 at 20:30
  • @user3087839 sendRequest and receiveResponse are both called when the servlet receives a JSON request. I am getting a response from the servlet only both requests get the same response what shouldn't happen. Is there a way I can call a specific thread, maybe by name? I ain't that far in programming yet, have been trying for around a month now so I have no idea what "classes holding state" means... Could you elaborate? –  Oct 20 '14 at 20:45
  • @user3087839 I have updated the code so I think now it should notify all threads however only if requestId matches it will continue. –  Oct 20 '14 at 21:14
  • [This](http://stackoverflow.com/questions/9735601/what-is-stateless-object-in-java) contains a succinct summary of stateful vs statelessness. – Floegipoky Oct 20 '14 at 21:50
  • 1
    I don't really understand what you're asking. There's way too much code here, you could eliminate probably half and still demonstrate the behavior you're interested in. Also, `Runnable` objects should not create their own threads. You should call `run` directly, or pass it into a thread or executor. It's especially inappropriate to do so in the constructor. – Floegipoky Oct 20 '14 at 22:10

1 Answers1

0

I have found a better(?) way to handle the thread communication I think, for the interested:

Global variable:

TreeMap<String, String> confirmResult = new TreeMap<String, String>();

First thread:

    Thread.currentThread().setName(requestId);
    try
    {
        synchronized(Thread.currentThread())
        {
            switch(serviceType)
            {
                case "GCM":
                    // Send notification to android device
                    GCM gcmClass = new GCM();
                    gcmClass.authenticateRequest(deviceId, requestId, apiKey);
                    break;
            }
            // Wait for reply
            Thread.currentThread().wait();
        }
        synchronized(this)
        {
            if(confirmResult.containsKey(requestId))
            {
                // Get the result
                String result = confirmResult.get(requestId);

                // Process the result
                switch(result)
                {
                    case "approved":
                        jsonResponse.setResult(0);
                        jsonResponse.setResultText("approved");
                        break;
                    case "cancelled":
                        jsonResponse.setResult(10000);
                        jsonResponse.setResultText("cancelled");
                        break;
                }
                // Remove the key from the TreeMap
                confirmResult.remove(requestId);
            }
        }
    }
    catch(Exception e)
    {
        e.printStackTrace();
    }

Second thread:

    synchronized(this)
    {
        // Put the result in the TreeMap
        confirmResult.put(requestId, confirmation);
    }
    // Get a list of all threads
    Map<Thread, StackTraceElement[]> arr = Thread.getAllStackTraces();
    // List the threads
    for(Map.Entry<Thread, StackTraceElement[]> entry : arr.entrySet())
    {
        // Check if the notify thread still exists
        if(entry.getKey().getName().equals(requestId))
        {
            synchronized(entry.getKey())
            {
                // Notify the thread
                entry.getKey().notify();
            }
        }
    }