Temporarily raise the number of system requests to 375

This is a stopgap to palliate a bad interaction between
ConnectivityService and JobScheduler.

To help developers find request leaks and protect against OOMs
in the system server, ConnectivityService is enforncing a maximum
number of network requests/callbacks per UID. It has a higher
value for system UIDs because some system components need to
file requests on behalf of apps, most prominently JobScheduler.

The count is increased on entry in ConnectivityService, on the
Binder thread ; this is when CS can throw TooManyRequestsException
to the caller, it could not do that later when the request is
actually registered on the handler thread.
Conversely it only decreases the count on the handler thread,
because until then it doesn't know whether the request was
actually registered, and it can't look at that data structure on
the Binder thread.
This leads to a slightly confusing result where a UID can register
a request immediately after it unregisters one, and it will receive
TooManyRequestsException, because the unregister hasn't been
processed yet and the count not decremented yet.
This is arguably a bug, but if your app is treading on that last
request, you almost certainly had a leak anyway, and you should
not be trying to play with the exact count of requests either.
The remaining case is when an app would unregister and register
requests again in a tight loop, but there is no valid use case for
that, right ? Right !?

Enter JobScheduler. Because the limit for system UIDs is 250
requests but there can be more than 250 UIDs (especially on
devices with multiple users), JobScheduler has its own limit of
requests (which is set to 125). It has heuristics to choose which
are the most important 125 UIDs to file requests for.

When the request count exceeds 125, or when something
happens that may change which are the most important 125
UIDs, JobScheduler will re-evaluate which are the most important
ones. Upon doing this, it will unregister all requests and file
again the important ones in a tight loop. At worst, it will
therefore remove 125 requests and add 125 requests again,
taking up at most all 250 requests allowed for system UIDs ;
if the system server has any other request outstanding (and
it does), it may hit the issue where CS doesn't know yet that
requests have been unregistered, and crashes and hilarity ensue.

To palliate the issue above, this patch increases the allowed
request count for system UIDs by 125. This immediately should
fix the issue where all 250 requests are taken up by JobScheduler.

For a long-term fix, we should instead make at least one of the
following fixes :
- Have JobScheduler be smarter, and make sure it only unregisters
  the requests that it no longer needs instead of all of them.
- Make CS only throw when the requests are really full, by using
  some combination of locking, making the caller wait or similar.
- Have a separate pool of requests for JobScheduler that is
  dimensioned so it knows it can file all the necessary requests
  and no longer has to limit itself, and simplify the JS code.

Flag: EXEMPT bugfix
Bug: 391114053
Test: ConnectivityServiceTest
Change-Id: I4c4702e2d0c5d03eb5f3d6fe0f1578a210b188e8
This commit is contained in:
Chalard Jean 2025-04-03 13:34:36 +09:00
commit ff3222ce51

View file

@ -504,8 +504,13 @@ public class ConnectivityService extends IConnectivityManager.Stub {
static final int MAX_NETWORK_REQUESTS_PER_UID = 100;
// The maximum number of network request allowed for system UIDs before an exception is thrown.
// The point of this limit is to help developers find request leaks, so it should be
// appropriately low, while allowing plenty of requests for normal use cases.
// In the case of the system UIDs, the main consumer of such requests is JobScheduler, which
// at most might file one request per UID on the system. Also see b/391114053 for some context
// about the specific JobScheduler case and its own self-imposed limit.
@VisibleForTesting
static final int MAX_NETWORK_REQUESTS_PER_SYSTEM_UID = 250;
static final int MAX_NETWORK_REQUESTS_PER_SYSTEM_UID = 375;
@VisibleForTesting
protected int mLingerDelayMs; // Can't be final, or test subclass constructors can't change it.