From 2d9864ab3cc00eea07f441dcdb0ed020d960bf36 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 28 Feb 2019 20:28:48 -0800 Subject: [PATCH] Fix remove-before-add for IpSecService RefcountedResource This patch fixes a bug where if a binder dies before the linkToDeath call, the cleanup will be performed before the entry is added to the array. While it is safe in that quotas and tracking performs as per normal, the RefcountedRecord may not be cleaned up. Rethrowing this exception is safe, since the only paths that would hit this are all on binder threads coming from applications. Further, it seems there is only one real way of this getting hit - if the app that called the creation died during the binder call. Bug: 126802451 Test: Compiled, CTS tests passing Change-Id: I6db75853da9f29e1573512e26351623f22770c5d --- .../server/IpSecServiceRefcountedResourceTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java b/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java index 68ff777a01..22a2c94fc1 100644 --- a/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java +++ b/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java @@ -18,6 +18,7 @@ package com.android.server; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.eq; @@ -134,11 +135,11 @@ public class IpSecServiceRefcountedResourceTest { IBinder binderMock = mock(IBinder.class); doThrow(new RemoteException()).when(binderMock).linkToDeath(anyObject(), anyInt()); - RefcountedResource refcountedResource = getTestRefcountedResource(binderMock); - - // Verify that cleanup is performed (Spy limitations prevent verification of method calls - // for binder death scenario; check refcount to determine if cleanup was performed.) - assertEquals(-1, refcountedResource.mRefCount); + try { + getTestRefcountedResource(binderMock); + fail("Expected exception to propogate when binder fails to link to death"); + } catch (RuntimeException expected) { + } } @Test