ethernet: fix receiving NetworkOffer callbacks on stale object

unregisterNetworkOffer() does not execute synchronously, so it is
possible that NetworkOffer callbacks are received after the offer was
unregistered.

Test: atest EthernetManagerTest && atest EthernetNetworkFactoryTest
Bug: 171872016
Change-Id: I2c50b01176e4968c31f83148f1831b55f8b06908
This commit is contained in:
Patrick Rohr
2022-07-11 14:20:50 -07:00
parent 1c37d3e7b0
commit 7c4e2a918c
2 changed files with 32 additions and 3 deletions

View File

@@ -293,7 +293,7 @@ public class EthernetNetworkFactory {
private final Context mContext; private final Context mContext;
private final NetworkProvider mNetworkProvider; private final NetworkProvider mNetworkProvider;
private final Dependencies mDeps; private final Dependencies mDeps;
private final NetworkProvider.NetworkOfferCallback mNetworkOfferCallback; private NetworkProvider.NetworkOfferCallback mNetworkOfferCallback;
private static String sTcpBufferSizes = null; // Lazy initialized. private static String sTcpBufferSizes = null; // Lazy initialized.
@@ -400,8 +400,15 @@ public class EthernetNetworkFactory {
} }
private class EthernetNetworkOfferCallback implements NetworkProvider.NetworkOfferCallback { private class EthernetNetworkOfferCallback implements NetworkProvider.NetworkOfferCallback {
private boolean isStale() {
return this != mNetworkOfferCallback;
}
@Override @Override
public void onNetworkNeeded(@NonNull NetworkRequest request) { public void onNetworkNeeded(@NonNull NetworkRequest request) {
if (isStale()) {
return;
}
if (DBG) { if (DBG) {
Log.d(TAG, String.format("%s: onNetworkNeeded for request: %s", name, request)); Log.d(TAG, String.format("%s: onNetworkNeeded for request: %s", name, request));
} }
@@ -416,6 +423,9 @@ public class EthernetNetworkFactory {
@Override @Override
public void onNetworkUnneeded(@NonNull NetworkRequest request) { public void onNetworkUnneeded(@NonNull NetworkRequest request) {
if (isStale()) {
return;
}
if (DBG) { if (DBG) {
Log.d(TAG, Log.d(TAG,
String.format("%s: onNetworkUnneeded for request: %s", name, request)); String.format("%s: onNetworkUnneeded for request: %s", name, request));
@@ -443,7 +453,6 @@ public class EthernetNetworkFactory {
mContext = context; mContext = context;
mNetworkProvider = networkProvider; mNetworkProvider = networkProvider;
mDeps = deps; mDeps = deps;
mNetworkOfferCallback = new EthernetNetworkOfferCallback();
mHwAddress = hwAddress; mHwAddress = hwAddress;
} }
@@ -669,13 +678,21 @@ public class EthernetNetworkFactory {
} }
private void registerNetworkOffer() { private void registerNetworkOffer() {
// If mNetworkOfferCallback is already set, it should be reused to update the existing
// offer.
if (mNetworkOfferCallback == null) {
mNetworkOfferCallback = new EthernetNetworkOfferCallback();
}
mNetworkProvider.registerNetworkOffer(getNetworkScore(), mNetworkProvider.registerNetworkOffer(getNetworkScore(),
new NetworkCapabilities(mCapabilities), cmd -> mHandler.post(cmd), new NetworkCapabilities(mCapabilities), cmd -> mHandler.post(cmd),
mNetworkOfferCallback); mNetworkOfferCallback);
} }
public void unregisterNetworkOfferAndStop() { private void unregisterNetworkOfferAndStop() {
mNetworkProvider.unregisterNetworkOffer(mNetworkOfferCallback); mNetworkProvider.unregisterNetworkOffer(mNetworkOfferCallback);
// Setting mNetworkOfferCallback to null allows the callback object to be identified
// as stale.
mNetworkOfferCallback = null;
stop(); stop();
mRequestIds.clear(); mRequestIds.clear();
} }

View File

@@ -726,4 +726,16 @@ public class EthernetNetworkFactoryTest {
triggerOnProvisioningSuccess(); triggerOnProvisioningSuccess();
verifyRestart(initialIpConfig); verifyRestart(initialIpConfig);
} }
@Test
public void testOnNetworkNeededOnStaleNetworkOffer() throws Exception {
initEthernetNetworkFactory();
createAndVerifyProvisionedInterface(TEST_IFACE);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, null);
verify(mNetworkProvider).unregisterNetworkOffer(mNetworkOfferCallback);
// It is possible that even after a network offer is unregistered, CS still sends it
// onNetworkNeeded() callbacks.
mNetworkOfferCallback.onNetworkNeeded(createDefaultRequest());
verify(mIpClient, never()).startProvisioning(any());
}
} }