Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ef9a2175 authored by Maksymilian Osowski's avatar Maksymilian Osowski
Browse files

Fixed a bug in forwarder where calling the stop method on ForwarderManager... 

Fixed a bug in forwarder where calling the stop method on ForwarderManager would deadlock. Also some style fixes.

The call would deadlock because lines 62-67 in ConnectionHandler were in the synchronized block. The is.read() would block and therefore call to shutdown() would block too.
is.read()
would never unblock because the stream was ready to be closed, and it needs to be done so from shutdown(). Removing the synchronized block fixes it and is save, as it was not
needed here.

Change-Id: I3326098c47ee18c4eabaa8510b27ca82debce360
parent c6f74139

tests/DumpRenderTree2/src/com/android/dumprendertree2/forwarder/ConnectionHandler.java

+17 −11
Original line number Diff line number Diff line
@@ -59,12 +59,10 @@ public class ConnectionHandler { 
            int length;

            while (true) {

                try {

                    synchronized (this) {

                    if ((length = is.read(buffer)) <= 0) {

                        break;

                    }

                    os.write(buffer, 0, length);

                    }

                } catch (IOException e) {

                    /** This exception means one of the streams is closed */

                    Log.v(LOG_TAG, this.toString(), e);
@@ -100,17 +98,25 @@ public class ConnectionHandler { 
    }



    private void shutdown(Socket socket) {

        try {

        synchronized (mFromToPipe) {

            synchronized (mToFromPipe) {

                /** This will stop the while loop in the run method */

                try {

                    socket.shutdownInput();

                    socket.shutdownOutput();

                    socket.close();

                } catch (IOException e) {

                    Log.e(LOG_TAG, "mFromToPipe=" + mFromToPipe + " mToFromPipe=" + mToFromPipe, e);

                }

                try {

                    socket.shutdownOutput();

                } catch (IOException e) {

                    Log.e(LOG_TAG, "mFromToPipe=" + mFromToPipe + " mToFromPipe=" + mToFromPipe, e);

                }

                try {

                    socket.close();

                } catch (IOException e) {

                    Log.e(LOG_TAG, "mFromToPipe=" + mFromToPipe + " mToFromPipe=" + mToFromPipe, e);

                }

            }

        }

    }

}
 
 No newline at end of file

tests/DumpRenderTree2/src/com/android/dumprendertree2/forwarder/Forwarder.java

+1 −13
Original line number Diff line number Diff line
@@ -47,12 +47,6 @@ public class Forwarder extends Thread { 
    @Override

    public void start() {

        Log.i(LOG_TAG, "start(): Starting fowarder on port: " + mPort);

        synchronized (this) {

            if (mIsRunning) {

                Log.w(LOG_TAG, "start(): Forwarder on port: " + mPort + " already running! NOOP.");

                return;

            }

        }



        try {

            mServerSocket = new ServerSocket(mPort);
@@ -82,7 +76,7 @@ public class Forwarder extends Thread { 
                            mPort);

                } catch (IOException e) {

                    /** This most likely means that mServerSocket is already closed */

                    Log.w(LOG_TAG + "mPort=" + mPort, e);

                    Log.w(LOG_TAG, "mPort=" + mPort, e);

                    return;

                }
@@ -107,12 +101,6 @@ public class Forwarder extends Thread { 
    }



    public void finish() {

        synchronized (this) {

            if (!mIsRunning) {

                return;

            }

        }



        try {

            mServerSocket.close();

        } catch (IOException e) {

tests/DumpRenderTree2/src/com/android/dumprendertree2/forwarder/ForwarderManager.java

+29 −8
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ package com.android.dumprendertree2.forwarder; 


import java.net.MalformedURLException;

import java.net.URL;

import android.util.Log;



import java.util.HashSet;

import java.util.Set;
@@ -27,6 +29,8 @@ import java.util.Set; 
 * It uses a singleton pattern and is thread safe.

 */

public class ForwarderManager {

    private static final String LOG_TAG = "ForwarderManager";



    /**

     * The IP address of the server serving the tests.

     */
@@ -43,12 +47,13 @@ public class ForwarderManager { 


    private static ForwarderManager forwarderManager;



    private Set<Forwarder> mServers;

    private Set<Forwarder> mForwarders;

    private boolean mIsStarted;



    private ForwarderManager() {

        mServers = new HashSet<Forwarder>(2);

        mServers.add(new Forwarder(HTTP_PORT, HOST_IP));

        mServers.add(new Forwarder(HTTPS_PORT, HOST_IP));

        mForwarders = new HashSet<Forwarder>(2);

        mForwarders.add(new Forwarder(HTTP_PORT, HOST_IP));

        mForwarders.add(new Forwarder(HTTPS_PORT, HOST_IP));

    }



    /**
@@ -91,14 +96,30 @@ public class ForwarderManager { 
    }



    public synchronized void start() {

        for (Forwarder server : mServers) {

            server.start();

        if (mIsStarted) {

            Log.w(LOG_TAG, "start(): ForwarderManager already running! NOOP.");

            return;

        }



        for (Forwarder forwarder : mForwarders) {

            forwarder.start();

        }



        mIsStarted = true;

        Log.i(LOG_TAG, "ForwarderManager started.");

    }



    public synchronized void stop() {

        for (Forwarder server : mServers) {

            server.finish();

        if (!mIsStarted) {

            Log.w(LOG_TAG, "stop(): ForwarderManager already stopped! NOOP.");

            return;

        }



        for (Forwarder forwarder : mForwarders) {

            forwarder.finish();

        }



        mIsStarted = false;

        Log.i(LOG_TAG, "ForwarderManager stopped.");

    }

}
 
 No newline at end of file