Skip to content

SecurityPolicy should support async/slow implementations #10566

@mateusazis

Description

@mateusazis

Is your feature request related to a problem?

The SecurityPolicy class defines an abstract method that returns a Status. If computing such status requires running some slow piece of code (that, for instance, reads flags from storage or from the network), then the only option is to block the calling thread until such operation is completed.

public class FlagBasedSecurityPolicy extends SecurityPolicy {
  @Override
  public Status checkAuthorization(int uid) {
    try {
      // must block the calling thread to satisfy SecurityPolicy's API
      return fetchAllowlistedUids().get().contains(uid) ? Status.OK : Status.PERMISSION_DENIED;
    } catch (InterruptedException e) {
      return Status.fromThrowable(e);
    }
  }

  // does not block
  private ListenableFuture<List<Integer>> fetchAllowlistedUids() {
    // ...
  }
}

Blocking is generally undesirable, because it monopolizes a thread from the system for mostly idle work, which leads to sub-optimal performance by requiring new threads to be spawned or reducing the amount of threads available, thus the responsiveness of the system.

Describe the solution you'd like

SecurityPolicy (or an equivalent replacement) should provide an API for returning a ListenableFuture<Status> instead. Clients are free to implement it however they see fit. The case commonly supported today (simple non-blocking computations) would be equivalent to returning a Futures.immediateFuture(status) (from Guava).

public interface AsyncSecurityPolicy {
  ListenableFuture<Status> checkAuthorization(int uid);
}

Then the code above can be written as non-blocking:

public class FlagBasedSecurityPolicy implements AsyncSecurityPolicy {
  @Override
  public ListenableFuture<Status> checkAuthorization(int uid) {
    return Futures.transform(
      fetchAllowlistedUids(),
      allowlist -> allowlist.contains(uid) ? Status.OK : Status.PERMISSION_DENIED,
      someExecutor);
  }

  // does not block
  private ListenableFuture<List<Integer>> fetchAllowlistedUids() {
    // ...
  }
}

Describe alternatives you've considered

N/A

Additional context

Some apps use StrictPolicy.VmPolicy to instruct which kinds of threads are appropriate for different kinds of tasks (e.g. lightweight computations vs blocking network access). It can be used to configure different "penalties", like just logging in production or crashing in development. The issue above has triggered such violations during the development of an internal Google app.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions