Skip to content

Commit d116d7c

Browse files
committed
Fixing memory leaks in the accessiiblity layer.
1. AccessibilityInteractionConnections were removed from the AccessiiblityManagerService but their DeathRecipents were not unregistered, thus every removed interaction connection was essentially leaking. Such connection is registered in the system for every ViewRootImpl when accessiiblity is enabled and inregistered when disabled. 2. Every AccessibilityEvent and AccessiilbityEventInfo obtained from a widnow content querying accessibility service had a handle to a binder proxy over which to make queries. Hoewever, holding a proxy to a remote binder prevents the latter from being garbage collected. Therefore, now the events and infos have a connection id insteand and the hindden singleton AccessiiblityInteaction client via which queries are made has a registry with the connections. This class looks up the connection given its id before making an IPC. Now the connection is stored in one place and when an accessibility service is disconnected the system sets the connection to null so the binder object in the system process can be GCed. Note that before this change a bad implemented accessibility service could cache events or infos causing a leak in the system process. This should never happen. 3. SparseArray was not clearing the reference to the last moved element while garbage collecting thus causing a leak. bug:5664337 Change-Id: Id397f614b026d43bd7b57bb7f8186bca5cdfcff9
1 parent 50b2042 commit d116d7c

File tree

10 files changed

+416
-312
lines changed

10 files changed

+416
-312
lines changed

core/java/android/accessibilityservice/AccessibilityService.java

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,18 @@
1616

1717
package android.accessibilityservice;
1818

19-
import com.android.internal.os.HandlerCaller;
20-
2119
import android.app.Service;
2220
import android.content.Intent;
2321
import android.os.IBinder;
2422
import android.os.Message;
2523
import android.os.RemoteException;
2624
import android.util.Log;
2725
import android.view.accessibility.AccessibilityEvent;
26+
import android.view.accessibility.AccessibilityInteractionClient;
2827
import android.view.accessibility.AccessibilityNodeInfo;
2928

29+
import com.android.internal.os.HandlerCaller;
30+
3031
/**
3132
* An accessibility service runs in the background and receives callbacks by the system
3233
* when {@link AccessibilityEvent}s are fired. Such events denote some state transition
@@ -219,7 +220,7 @@ public abstract class AccessibilityService extends Service {
219220

220221
private AccessibilityServiceInfo mInfo;
221222

222-
IAccessibilityServiceConnection mConnection;
223+
private int mConnectionId;
223224

224225
/**
225226
* Callback for {@link android.view.accessibility.AccessibilityEvent}s.
@@ -264,9 +265,11 @@ public final void setServiceInfo(AccessibilityServiceInfo info) {
264265
* AccessibilityManagerService.
265266
*/
266267
private void sendServiceInfo() {
267-
if (mInfo != null && mConnection != null) {
268+
IAccessibilityServiceConnection connection =
269+
AccessibilityInteractionClient.getInstance().getConnection(mConnectionId);
270+
if (mInfo != null && connection != null) {
268271
try {
269-
mConnection.setServiceInfo(mInfo);
272+
connection.setServiceInfo(mInfo);
270273
} catch (RemoteException re) {
271274
Log.w(LOG_TAG, "Error while setting AccessibilityServiceInfo", re);
272275
}
@@ -302,8 +305,9 @@ public IEventListenerWrapper(AccessibilityService context) {
302305
mCaller = new HandlerCaller(context, this);
303306
}
304307

305-
public void setConnection(IAccessibilityServiceConnection connection) {
306-
Message message = mCaller.obtainMessageO(DO_SET_SET_CONNECTION, connection);
308+
public void setConnection(IAccessibilityServiceConnection connection, int connectionId) {
309+
Message message = mCaller.obtainMessageIO(DO_SET_SET_CONNECTION, connectionId,
310+
connection);
307311
mCaller.sendMessage(message);
308312
}
309313

@@ -330,8 +334,19 @@ public void executeMessage(Message message) {
330334
mTarget.onInterrupt();
331335
return;
332336
case DO_SET_SET_CONNECTION :
333-
mConnection = ((IAccessibilityServiceConnection) message.obj);
334-
mTarget.onServiceConnected();
337+
final int connectionId = message.arg1;
338+
IAccessibilityServiceConnection connection =
339+
(IAccessibilityServiceConnection) message.obj;
340+
if (connection != null) {
341+
AccessibilityInteractionClient.getInstance().addConnection(connectionId,
342+
connection);
343+
mConnectionId = connectionId;
344+
mTarget.onServiceConnected();
345+
} else {
346+
AccessibilityInteractionClient.getInstance().removeConnection(connectionId);
347+
mConnectionId = AccessibilityInteractionClient.NO_ID;
348+
// TODO: Do we need a onServiceDisconnected callback?
349+
}
335350
return;
336351
default :
337352
Log.w(LOG_TAG, "Unknown message type " + message.what);

core/java/android/accessibilityservice/IEventListener.aidl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import android.view.accessibility.AccessibilityEvent;
2626
*/
2727
oneway interface IEventListener {
2828

29-
void setConnection(in IAccessibilityServiceConnection connection);
29+
void setConnection(in IAccessibilityServiceConnection connection, int connectionId);
3030

3131
void onAccessibilityEvent(in AccessibilityEvent event);
3232

core/java/android/util/SparseArray.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ private void gc() {
134134
if (i != o) {
135135
keys[o] = keys[i];
136136
values[o] = val;
137+
values[i] = null;
137138
}
138139

139140
o++;

core/java/android/view/accessibility/AccessibilityEvent.java

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package android.view.accessibility;
1818

19-
import android.accessibilityservice.IAccessibilityServiceConnection;
2019
import android.os.Parcel;
2120
import android.os.Parcelable;
2221
import android.text.TextUtils;
@@ -589,24 +588,6 @@ void init(AccessibilityEvent event) {
589588
mPackageName = event.mPackageName;
590589
}
591590

592-
/**
593-
* Sets the connection for interacting with the AccessibilityManagerService.
594-
*
595-
* @param connection The connection.
596-
*
597-
* @hide
598-
*/
599-
@Override
600-
public void setConnection(IAccessibilityServiceConnection connection) {
601-
super.setConnection(connection);
602-
List<AccessibilityRecord> records = mRecords;
603-
final int recordCount = records.size();
604-
for (int i = 0; i < recordCount; i++) {
605-
AccessibilityRecord record = records.get(i);
606-
record.setConnection(connection);
607-
}
608-
}
609-
610591
/**
611592
* Sets if this instance is sealed.
612593
*
@@ -821,23 +802,19 @@ protected void clear() {
821802
* @param parcel A parcel containing the state of a {@link AccessibilityEvent}.
822803
*/
823804
public void initFromParcel(Parcel parcel) {
824-
if (parcel.readInt() == 1) {
825-
mConnection = IAccessibilityServiceConnection.Stub.asInterface(
826-
parcel.readStrongBinder());
827-
}
828-
setSealed(parcel.readInt() == 1);
805+
mSealed = (parcel.readInt() == 1);
829806
mEventType = parcel.readInt();
830807
mPackageName = TextUtils.CHAR_SEQUENCE_CREATOR.createFromParcel(parcel);
831808
mEventTime = parcel.readLong();
809+
mConnectionId = parcel.readInt();
832810
readAccessibilityRecordFromParcel(this, parcel);
833811

834812
// Read the records.
835813
final int recordCount = parcel.readInt();
836814
for (int i = 0; i < recordCount; i++) {
837815
AccessibilityRecord record = AccessibilityRecord.obtain();
838-
// Do this to write the connection only once.
839-
record.setConnection(mConnection);
840816
readAccessibilityRecordFromParcel(record, parcel);
817+
record.mConnectionId = mConnectionId;
841818
mRecords.add(record);
842819
}
843820
}
@@ -875,16 +852,11 @@ private void readAccessibilityRecordFromParcel(AccessibilityRecord record,
875852
* {@inheritDoc}
876853
*/
877854
public void writeToParcel(Parcel parcel, int flags) {
878-
if (mConnection == null) {
879-
parcel.writeInt(0);
880-
} else {
881-
parcel.writeInt(1);
882-
parcel.writeStrongBinder(mConnection.asBinder());
883-
}
884855
parcel.writeInt(isSealed() ? 1 : 0);
885856
parcel.writeInt(mEventType);
886857
TextUtils.writeToParcel(mPackageName, parcel, 0);
887858
parcel.writeLong(mEventTime);
859+
parcel.writeInt(mConnectionId);
888860
writeAccessibilityRecordToParcel(this, parcel, flags);
889861

890862
// Write the records.

0 commit comments

Comments
 (0)