Fix how WifiMonitor/WifiP2pMonitor sends a message
A message can potentially have multiple handlers. Looper recycles
the message after one handler is done with it. Sending/copying the
recycled message to another handler is problematic. Fix the issue
by always sending a copy of the original message.
Bug: 37714369
Test: runtest.sh
Change-Id: I2f4a4903ac8da94816341def969878f888154b6c
(cherry picked from commit 098640f533f3fc04cb49cb860f9dc45ad8a19e69)
diff --git a/service/java/com/android/server/wifi/WifiMonitor.java b/service/java/com/android/server/wifi/WifiMonitor.java
index d1903a0..385bfcc 100644
--- a/service/java/com/android/server/wifi/WifiMonitor.java
+++ b/service/java/com/android/server/wifi/WifiMonitor.java
@@ -262,14 +262,10 @@
SparseArray<Set<Handler>> ifaceHandlers = mHandlerMap.get(iface);
if (iface != null && ifaceHandlers != null) {
if (isMonitoring(iface)) {
- boolean firstHandler = true;
Set<Handler> ifaceWhatHandlers = ifaceHandlers.get(message.what);
if (ifaceWhatHandlers != null) {
for (Handler handler : ifaceWhatHandlers) {
- if (firstHandler) {
- firstHandler = false;
- sendMessage(handler, message);
- } else {
+ if (handler != null) {
sendMessage(handler, Message.obtain(message));
}
}
@@ -283,28 +279,24 @@
if (mVerboseLoggingEnabled) {
Log.d(TAG, "Sending to all monitors because there's no matching iface");
}
- boolean firstHandler = true;
for (Map.Entry<String, SparseArray<Set<Handler>>> entry : mHandlerMap.entrySet()) {
if (isMonitoring(entry.getKey())) {
Set<Handler> ifaceWhatHandlers = entry.getValue().get(message.what);
for (Handler handler : ifaceWhatHandlers) {
- if (firstHandler) {
- firstHandler = false;
- sendMessage(handler, message);
- } else {
+ if (handler != null) {
sendMessage(handler, Message.obtain(message));
}
}
}
}
}
+
+ message.recycle();
}
private void sendMessage(Handler handler, Message message) {
- if (handler != null) {
- message.setTarget(handler);
- message.sendToTarget();
- }
+ message.setTarget(handler);
+ message.sendToTarget();
}
/**
diff --git a/service/java/com/android/server/wifi/p2p/WifiP2pMonitor.java b/service/java/com/android/server/wifi/p2p/WifiP2pMonitor.java
index 50fd932..0411f30 100644
--- a/service/java/com/android/server/wifi/p2p/WifiP2pMonitor.java
+++ b/service/java/com/android/server/wifi/p2p/WifiP2pMonitor.java
@@ -240,14 +240,10 @@
SparseArray<Set<Handler>> ifaceHandlers = mHandlerMap.get(iface);
if (iface != null && ifaceHandlers != null) {
if (isMonitoring(iface)) {
- boolean firstHandler = true;
Set<Handler> ifaceWhatHandlers = ifaceHandlers.get(message.what);
if (ifaceWhatHandlers != null) {
for (Handler handler : ifaceWhatHandlers) {
- if (firstHandler) {
- firstHandler = false;
- sendMessage(handler, message);
- } else {
+ if (handler != null) {
sendMessage(handler, Message.obtain(message));
}
}
@@ -261,28 +257,24 @@
if (mVerboseLoggingEnabled) {
Log.d(TAG, "Sending to all monitors because there's no matching iface");
}
- boolean firstHandler = true;
for (Map.Entry<String, SparseArray<Set<Handler>>> entry : mHandlerMap.entrySet()) {
if (isMonitoring(entry.getKey())) {
Set<Handler> ifaceWhatHandlers = entry.getValue().get(message.what);
for (Handler handler : ifaceWhatHandlers) {
- if (firstHandler) {
- firstHandler = false;
- sendMessage(handler, message);
- } else {
+ if (handler != null) {
sendMessage(handler, Message.obtain(message));
}
}
}
}
}
+
+ message.recycle();
}
private void sendMessage(Handler handler, Message message) {
- if (handler != null) {
- message.setTarget(handler);
- message.sendToTarget();
- }
+ message.setTarget(handler);
+ message.sendToTarget();
}
/**
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiMonitorTest.java b/tests/wifitests/src/com/android/server/wifi/WifiMonitorTest.java
index 4b4a17d..64d7c87 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiMonitorTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiMonitorTest.java
@@ -48,6 +48,7 @@
@SmallTest
public class WifiMonitorTest {
private static final String WLAN_IFACE_NAME = "wlan0";
+ private static final String SECOND_WLAN_IFACE_NAME = "wlan1";
private static final String[] GSM_AUTH_DATA = { "45adbc", "fead45", "0x3452"};
private static final String[] UMTS_AUTH_DATA = { "fead45", "0x3452"};
private static final String BSSID = "fe:45:23:12:12:0a";
@@ -56,12 +57,14 @@
private WifiMonitor mWifiMonitor;
private TestLooper mLooper;
private Handler mHandlerSpy;
+ private Handler mSecondHandlerSpy;
@Before
public void setUp() throws Exception {
mWifiMonitor = new WifiMonitor(mock(WifiInjector.class));
mLooper = new TestLooper();
mHandlerSpy = spy(new Handler(mLooper.getLooper()));
+ mSecondHandlerSpy = spy(new Handler(mLooper.getLooper()));
mWifiMonitor.setMonitoring(WLAN_IFACE_NAME, true);
}
@@ -485,4 +488,50 @@
verify(mHandlerSpy).handleMessage(messageCaptor.capture());
assertEquals(WifiMonitor.SUP_DISCONNECTION_EVENT, messageCaptor.getValue().what);
}
+ /**
+ * Broadcast message to two handlers test.
+ */
+ @Test
+ public void testBroadcastEventToTwoHandlers() {
+ mWifiMonitor.registerHandler(
+ WLAN_IFACE_NAME, WifiMonitor.SUP_CONNECTION_EVENT, mHandlerSpy);
+ mWifiMonitor.registerHandler(
+ WLAN_IFACE_NAME, WifiMonitor.SUP_CONNECTION_EVENT, mSecondHandlerSpy);
+ mWifiMonitor.broadcastSupplicantConnectionEvent(WLAN_IFACE_NAME);
+ mLooper.dispatchAll();
+
+ ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+ verify(mHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiMonitor.SUP_CONNECTION_EVENT, messageCaptor.getValue().what);
+ verify(mSecondHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiMonitor.SUP_CONNECTION_EVENT, messageCaptor.getValue().what);
+ }
+ /**
+ * Broadcast message when iface is null.
+ */
+ @Test
+ public void testBroadcastEventWhenIfaceIsNull() {
+ mWifiMonitor.registerHandler(
+ WLAN_IFACE_NAME, WifiMonitor.SUP_DISCONNECTION_EVENT, mHandlerSpy);
+ mWifiMonitor.broadcastSupplicantDisconnectionEvent(null);
+ mLooper.dispatchAll();
+
+ ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+ verify(mHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiMonitor.SUP_DISCONNECTION_EVENT, messageCaptor.getValue().what);
+ }
+ /**
+ * Broadcast message when iface handler is null.
+ */
+ @Test
+ public void testBroadcastEventWhenIfaceHandlerIsNull() {
+ mWifiMonitor.registerHandler(
+ WLAN_IFACE_NAME, WifiMonitor.SUP_DISCONNECTION_EVENT, mHandlerSpy);
+ mWifiMonitor.broadcastSupplicantDisconnectionEvent(SECOND_WLAN_IFACE_NAME);
+ mLooper.dispatchAll();
+
+ ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+ verify(mHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiMonitor.SUP_DISCONNECTION_EVENT, messageCaptor.getValue().what);
+ }
}
diff --git a/tests/wifitests/src/com/android/server/wifi/p2p/WifiP2pMonitorTest.java b/tests/wifitests/src/com/android/server/wifi/p2p/WifiP2pMonitorTest.java
new file mode 100644
index 0000000..c2c3473
--- /dev/null
+++ b/tests/wifitests/src/com/android/server/wifi/p2p/WifiP2pMonitorTest.java
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.wifi.p2p;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
+import android.os.Handler;
+import android.os.Message;
+import android.os.test.TestLooper;
+import android.test.suitebuilder.annotation.SmallTest;
+
+import com.android.server.wifi.WifiInjector;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+/**
+ * Unit tests for {@link com.android.server.wifi.WifiP2pMonitor}.
+ */
+@SmallTest
+public class WifiP2pMonitorTest {
+ private static final String P2P_IFACE_NAME = "p2p0";
+ private static final String SECOND_P2P_IFACE_NAME = "p2p1";
+ private WifiP2pMonitor mWifiP2pMonitor;
+ private TestLooper mLooper;
+ private Handler mHandlerSpy;
+ private Handler mSecondHandlerSpy;
+
+ @Before
+ public void setUp() throws Exception {
+ mWifiP2pMonitor = new WifiP2pMonitor(mock(WifiInjector.class));
+ mLooper = new TestLooper();
+ mHandlerSpy = spy(new Handler(mLooper.getLooper()));
+ mSecondHandlerSpy = spy(new Handler(mLooper.getLooper()));
+ mWifiP2pMonitor.setMonitoring(P2P_IFACE_NAME, true);
+ }
+
+ /**
+ * Broadcast message test.
+ */
+ @Test
+ public void testBroadcastSupplicantDisconnectionEvent() {
+ mWifiP2pMonitor.registerHandler(
+ P2P_IFACE_NAME, WifiP2pMonitor.SUP_DISCONNECTION_EVENT, mHandlerSpy);
+ mWifiP2pMonitor.broadcastSupplicantDisconnectionEvent(P2P_IFACE_NAME);
+ mLooper.dispatchAll();
+
+ ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+ verify(mHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiP2pMonitor.SUP_DISCONNECTION_EVENT, messageCaptor.getValue().what);
+ }
+ /**
+ * Broadcast message to two handlers test.
+ */
+ @Test
+ public void testBroadcastEventToTwoHandlers() {
+ mWifiP2pMonitor.registerHandler(
+ P2P_IFACE_NAME, WifiP2pMonitor.SUP_CONNECTION_EVENT, mHandlerSpy);
+ mWifiP2pMonitor.registerHandler(
+ P2P_IFACE_NAME, WifiP2pMonitor.SUP_CONNECTION_EVENT, mSecondHandlerSpy);
+ mWifiP2pMonitor.broadcastSupplicantConnectionEvent(P2P_IFACE_NAME);
+ mLooper.dispatchAll();
+
+ ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+ verify(mHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiP2pMonitor.SUP_CONNECTION_EVENT, messageCaptor.getValue().what);
+ verify(mSecondHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiP2pMonitor.SUP_CONNECTION_EVENT, messageCaptor.getValue().what);
+ }
+ /**
+ * Broadcast message when iface is null.
+ */
+ @Test
+ public void testBroadcastEventWhenIfaceIsNull() {
+ mWifiP2pMonitor.registerHandler(
+ P2P_IFACE_NAME, WifiP2pMonitor.SUP_DISCONNECTION_EVENT, mHandlerSpy);
+ mWifiP2pMonitor.broadcastSupplicantDisconnectionEvent(null);
+ mLooper.dispatchAll();
+
+ ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+ verify(mHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiP2pMonitor.SUP_DISCONNECTION_EVENT, messageCaptor.getValue().what);
+ }
+ /**
+ * Broadcast message when iface handler is null.
+ */
+ @Test
+ public void testBroadcastEventWhenIfaceHandlerIsNull() {
+ mWifiP2pMonitor.registerHandler(
+ P2P_IFACE_NAME, WifiP2pMonitor.SUP_DISCONNECTION_EVENT, mHandlerSpy);
+ mWifiP2pMonitor.broadcastSupplicantDisconnectionEvent(SECOND_P2P_IFACE_NAME);
+ mLooper.dispatchAll();
+
+ ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+ verify(mHandlerSpy).handleMessage(messageCaptor.capture());
+ assertEquals(WifiP2pMonitor.SUP_DISCONNECTION_EVENT, messageCaptor.getValue().what);
+ }
+}
