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

Commit 3f94640e authored by Narayan Kamath's avatar Narayan Kamath
Browse files

Fix a few synchronization issues in WifiMonitor.

- Guard all field accesses in WifiMonitorSingleton with
  a lock.
- WifiMonitorSingleton is now responsible for dispatching
  events to a given monitor (or all monitors if it can't
  find a matching monitor).
- Individual WifiMonitors are now responsible for dispatching
  events sent to them. This makes WifiMonitorThread a dumb
  object. All it does is wait for events and feed them back
  into the WifiMonitorSingleton.
- Also fixes a bug where we weren't telling the WifiMonitor
  that we're disconnected and another where we don't check whether
  a monitor is connected or not before asking it to dispatch
  an event.
- Also, replaces a few uses of entrySet() with a values() iterator
  when the keys are never used. The performance of both methods
  is identical for a HashMap, but the latter is a bit more concise
  and easier to read.

Change-Id: I7ce00174a78c72836666d25ccc5e6e9e687c2570
parent 9ddfa0e2
Loading
Loading
Loading
Loading
+415 −418
Original line number Original line Diff line number Diff line
@@ -26,14 +26,11 @@ import android.net.wifi.p2p.nsd.WifiP2pServiceResponse;
import android.os.Message;
import android.os.Message;
import android.util.Log;
import android.util.Log;



import com.android.internal.util.Protocol;
import com.android.internal.util.Protocol;
import com.android.internal.util.StateMachine;
import com.android.internal.util.StateMachine;


import java.util.HashMap;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.regex.Pattern;


@@ -335,10 +332,6 @@ public class WifiMonitor {


    /* Indicates assoc reject event */
    /* Indicates assoc reject event */
    public static final int ASSOCIATION_REJECTION_EVENT          = BASE + 43;
    public static final int ASSOCIATION_REJECTION_EVENT          = BASE + 43;
    /**
     * This indicates the supplicant connection for the monitor is closed
     */
    private static final String MONITOR_SOCKET_CLOSED_STR = "connection closed";


    /**
    /**
     * This indicates a read error on the monitor socket conenction
     * This indicates a read error on the monitor socket conenction
@@ -352,14 +345,26 @@ public class WifiMonitor {


    private final String mInterfaceName;
    private final String mInterfaceName;
    private final WifiNative mWifiNative;
    private final WifiNative mWifiNative;
    private final StateMachine mWifiStateMachine;
    private final StateMachine mStateMachine;
    private boolean mMonitoring;
    private boolean mMonitoring;


    // This is a global counter, since it's not monitor specific. However, the existing
    // implementation forwards all "global" control events like CTRL-EVENT-TERMINATING
    // to the p2p0 monitor. Is that expected ? It seems a bit surprising.
    //
    // TODO: If the p2p0 monitor isn't registered, the behaviour is even more surprising.
    // The event will be dispatched to all monitors, and each of them will end up incrementing
    // it in their dispatchXXX method. If we have 5 registered monitors (say), 2 consecutive
    // recv errors will cause us to disconnect from the supplicant (instead of the intended 10).
    //
    // This variable is always accessed and modified under a WifiMonitorSingleton lock.
    private static int sRecvErrors;

    public WifiMonitor(StateMachine wifiStateMachine, WifiNative wifiNative) {
    public WifiMonitor(StateMachine wifiStateMachine, WifiNative wifiNative) {
        if (DBG) Log.d(TAG, "Creating WifiMonitor");
        if (DBG) Log.d(TAG, "Creating WifiMonitor");
        mWifiNative = wifiNative;
        mWifiNative = wifiNative;
        mInterfaceName = wifiNative.mInterfaceName;
        mInterfaceName = wifiNative.mInterfaceName;
        mWifiStateMachine = wifiStateMachine;
        mStateMachine = wifiStateMachine;
        mMonitoring = false;
        mMonitoring = false;


        WifiMonitorSingleton.sInstance.registerInterfaceMonitor(mInterfaceName, this);
        WifiMonitorSingleton.sInstance.registerInterfaceMonitor(mInterfaceName, this);
@@ -402,14 +407,14 @@ public class WifiMonitor {


            if (mConnected) {
            if (mConnected) {
                m.mMonitoring = true;
                m.mMonitoring = true;
                m.mWifiStateMachine.sendMessage(SUP_CONNECTION_EVENT);
                m.mStateMachine.sendMessage(SUP_CONNECTION_EVENT);
            } else {
            } else {
                if (DBG) Log.d(TAG, "connecting to supplicant");
                if (DBG) Log.d(TAG, "connecting to supplicant");
                int connectTries = 0;
                int connectTries = 0;
                while (true) {
                while (true) {
                    if (mWifiNative.connectToSupplicant()) {
                    if (mWifiNative.connectToSupplicant()) {
                        m.mMonitoring = true;
                        m.mMonitoring = true;
                        m.mWifiStateMachine.sendMessage(SUP_CONNECTION_EVENT);
                        m.mStateMachine.sendMessage(SUP_CONNECTION_EVENT);
                        new MonitorThread(mWifiNative, this).start();
                        new MonitorThread(mWifiNative, this).start();
                        mConnected = true;
                        mConnected = true;
                        break;
                        break;
@@ -421,7 +426,7 @@ public class WifiMonitor {
                        }
                        }
                    } else {
                    } else {
                        mIfaceMap.remove(iface);
                        mIfaceMap.remove(iface);
                        m.mWifiStateMachine.sendMessage(SUP_DISCONNECTION_EVENT);
                        m.mStateMachine.sendMessage(SUP_DISCONNECTION_EVENT);
                        Log.e(TAG, "startMonitoring(" + iface + ") failed!");
                        Log.e(TAG, "startMonitoring(" + iface + ") failed!");
                        break;
                        break;
                    }
                    }
@@ -431,13 +436,13 @@ public class WifiMonitor {


        public synchronized void stopMonitoring(String iface) {
        public synchronized void stopMonitoring(String iface) {
            WifiMonitor m = mIfaceMap.get(iface);
            WifiMonitor m = mIfaceMap.get(iface);
            if (DBG) Log.d(TAG, "stopMonitoring(" + iface + ") = " + m.mWifiStateMachine);
            if (DBG) Log.d(TAG, "stopMonitoring(" + iface + ") = " + m.mStateMachine);
            m.mMonitoring = false;
            m.mMonitoring = false;
            m.mWifiStateMachine.sendMessage(SUP_DISCONNECTION_EVENT);
            m.mStateMachine.sendMessage(SUP_DISCONNECTION_EVENT);
        }
        }


        public synchronized void registerInterfaceMonitor(String iface, WifiMonitor m) {
        public synchronized void registerInterfaceMonitor(String iface, WifiMonitor m) {
            if (DBG) Log.d(TAG, "registerInterface(" + iface + "+" + m.mWifiStateMachine + ")");
            if (DBG) Log.d(TAG, "registerInterface(" + iface + "+" + m.mStateMachine + ")");
            mIfaceMap.put(iface, m);
            mIfaceMap.put(iface, m);
            if (mWifiNative == null) {
            if (mWifiNative == null) {
                mWifiNative = m.mWifiNative;
                mWifiNative = m.mWifiNative;
@@ -449,7 +454,7 @@ public class WifiMonitor {
            // objects will remain in the mIfaceMap; and won't ever get deleted
            // objects will remain in the mIfaceMap; and won't ever get deleted


            WifiMonitor m = mIfaceMap.remove(iface);
            WifiMonitor m = mIfaceMap.remove(iface);
            if (DBG) Log.d(TAG, "unregisterInterface(" + iface + "+" + m.mWifiStateMachine + ")");
            if (DBG) Log.d(TAG, "unregisterInterface(" + iface + "+" + m.mStateMachine + ")");
        }
        }


        public synchronized void stopSupplicant() {
        public synchronized void stopSupplicant() {
@@ -457,100 +462,93 @@ public class WifiMonitor {
        }
        }


        public synchronized void killSupplicant(boolean p2pSupported) {
        public synchronized void killSupplicant(boolean p2pSupported) {
            mWifiNative.killSupplicant(p2pSupported);
            WifiNative.killSupplicant(p2pSupported);
            mConnected = false;
            mConnected = false;
            Iterator<Map.Entry<String, WifiMonitor>> it = mIfaceMap.entrySet().iterator();
            for (WifiMonitor m : mIfaceMap.values()) {
            while (it.hasNext()) {
                Map.Entry<String, WifiMonitor> e = it.next();
                WifiMonitor m = e.getValue();
                m.mMonitoring = false;
                m.mMonitoring = false;
            }
            }
        }
        }


        private synchronized WifiMonitor getMonitor(String iface) {
        private synchronized boolean dispatchEvent(String eventStr) {
            return mIfaceMap.get(iface);
            String iface;
        }
    }

    private static class MonitorThread extends Thread {
        private final WifiNative mWifiNative;
        private final WifiMonitorSingleton mWifiMonitorSingleton;
        private int mRecvErrors = 0;
        private StateMachine mStateMachine = null;

        public MonitorThread(WifiNative wifiNative, WifiMonitorSingleton wifiMonitorSingleton) {
            super("WifiMonitor");
            mWifiNative = wifiNative;
            mWifiMonitorSingleton = wifiMonitorSingleton;
        }

        public void run() {
            //noinspection InfiniteLoopStatement
            for (;;) {
                String eventStr = mWifiNative.waitForEvent();

                // Skip logging the common but mostly uninteresting scan-results event
                if (DBG && eventStr.indexOf(SCAN_RESULTS_STR) == -1) {
                    Log.d(TAG, "Event [" + eventStr + "]");
                }

                String iface = "p2p0";
                WifiMonitor m = null;
                mStateMachine = null;

            if (eventStr.startsWith("IFNAME=")) {
            if (eventStr.startsWith("IFNAME=")) {
                int space = eventStr.indexOf(' ');
                int space = eventStr.indexOf(' ');
                if (space != -1) {
                if (space != -1) {
                    iface = eventStr.substring(7, space);
                    iface = eventStr.substring(7, space);
                        m = mWifiMonitorSingleton.getMonitor(iface);
                    if (!mIfaceMap.containsKey(iface) && iface.startsWith("p2p-")) {
                        if (m == null && iface.startsWith("p2p-")) {
                        // p2p interfaces are created dynamically, but we have
                        // p2p interfaces are created dynamically, but we have
                        // only one P2p state machine monitoring all of them; look
                        // only one P2p state machine monitoring all of them; look
                        // for it explicitly, and send messages there ..
                        // for it explicitly, and send messages there ..
                            m = mWifiMonitorSingleton.getMonitor("p2p0");
                        iface = "p2p0";
                    }
                    }
                    eventStr = eventStr.substring(space + 1);
                    eventStr = eventStr.substring(space + 1);
                } else {
                    // No point dispatching this event to any interface, the dispatched
                    // event string will begin with "IFNAME=" which dispatchEvent can't really
                    // do anything about.
                    Log.e(TAG, "Dropping malformed event (unparsable iface): " + eventStr);
                    return false;
                }
                }
            } else {
            } else {
                // events without prefix belong to p2p0 monitor
                // events without prefix belong to p2p0 monitor
                    m = mWifiMonitorSingleton.getMonitor("p2p0");
                iface = "p2p0";
            }
            }


            if (DBG) Log.d(TAG, "Dispatching event to interface: " + iface);

            WifiMonitor m = mIfaceMap.get(iface);
            if (m != null) {
            if (m != null) {
                if (m.mMonitoring) {
                if (m.mMonitoring) {
                        mStateMachine = m.mWifiStateMachine;
                    if (m.dispatchEvent(eventStr)) {
                    } else {
                        mConnected = false;
                        if (DBG) Log.d(TAG, "Dropping event because monitor (" + iface +
                        return true;
                                            ") is stopped");
                        continue;
                    }
                    }
                    }


                if (mStateMachine != null) {
                    return false;
                    if (dispatchEvent(eventStr)) {
                } else {
                        break;
                    if (DBG) Log.d(TAG, "Dropping event because (" + iface + ") is stopped");
                    return false;
                }
                }
            } else {
            } else {
                    if (DBG) Log.d(TAG, "Sending to all monitors because there's no interface id");
                if (DBG) Log.d(TAG, "Sending to all monitors because there's no matching iface");
                boolean done = false;
                boolean done = false;
                    // TODO: This isn't thread safe, mIfaceMap & mConnected below are
                for (WifiMonitor monitor : mIfaceMap.values()) {
                    // usually guarded by (WifiMonitorSingleton.sInstance.this).
                    if (monitor.mMonitoring && monitor.dispatchEvent(eventStr)) {
                    Iterator<Map.Entry<String, WifiMonitor>> it =
                            mWifiMonitorSingleton.mIfaceMap.entrySet().iterator();
                    while (it.hasNext()) {
                        Map.Entry<String, WifiMonitor> e = it.next();
                        m = e.getValue();
                        mStateMachine = m.mWifiStateMachine;
                        if (dispatchEvent(eventStr)) {
                        done = true;
                        done = true;
                    }
                    }
                }
                }


                if (done) {
                if (done) {
                        // After this thread terminates, we'll no longer
                    mConnected = false;
                        // be connected to the supplicant
                }

                return done;
            }
        }
    }

    private static class MonitorThread extends Thread {
        private final WifiNative mWifiNative;
        private final WifiMonitorSingleton mWifiMonitorSingleton;

        public MonitorThread(WifiNative wifiNative, WifiMonitorSingleton wifiMonitorSingleton) {
            super("WifiMonitor");
            mWifiNative = wifiNative;
            mWifiMonitorSingleton = wifiMonitorSingleton;
        }

        public void run() {
            //noinspection InfiniteLoopStatement
            for (;;) {
                String eventStr = mWifiNative.waitForEvent();

                // Skip logging the common but mostly uninteresting scan-results event
                if (DBG && eventStr.indexOf(SCAN_RESULTS_STR) == -1) {
                    Log.d(TAG, "Event [" + eventStr + "]");
                }

                if (mWifiMonitorSingleton.dispatchEvent(eventStr)) {
                    if (DBG) Log.d(TAG, "Disconnecting from the supplicant, no more events");
                    if (DBG) Log.d(TAG, "Disconnecting from the supplicant, no more events");
                        mWifiMonitorSingleton.mConnected = false;
                    break;
                    break;
                }
                }
            }
            }
@@ -641,7 +639,7 @@ public class WifiMonitor {
             * too many recv errors
             * too many recv errors
             */
             */
            if (eventData.startsWith(WPA_RECV_ERROR_STR)) {
            if (eventData.startsWith(WPA_RECV_ERROR_STR)) {
                    if (++mRecvErrors > MAX_RECV_ERRORS) {
                if (++sRecvErrors > MAX_RECV_ERRORS) {
                    if (DBG) {
                    if (DBG) {
                        Log.d(TAG, "too many recv errors, closing connection");
                        Log.d(TAG, "too many recv errors, closing connection");
                    }
                    }
@@ -662,7 +660,7 @@ public class WifiMonitor {
        } else {
        } else {
            handleEvent(event, eventData);
            handleEvent(event, eventData);
        }
        }
            mRecvErrors = 0;
        sRecvErrors = 0;
        return false;
        return false;
    }
    }


@@ -936,4 +934,3 @@ public class WifiMonitor {
                new StateChangeResult(networkId, wifiSsid, BSSID, newState)));
                new StateChangeResult(networkId, wifiSsid, BSSID, newState)));
    }
    }
}
}
}