google-oauth-java-client: AbstractAuthorizationCodeCallbackServlet is blocked because of locks

[krog78]

Environment details

https://github.com/googleapis/google-oauth-java-client/blob/main/google-oauth-client-servlet/src/main/java/com/google/api/client/extensions/servlet/auth/oauth2/AbstractAuthorizationCodeCallbackServlet.java

Steps to reproduce

1. Send multiple times by second request to the callback URL
2. Threads are blocked because of the ReentrantLock()

Code example

/** Lock on the flow. */
  private final Lock lock = new ReentrantLock();

  /**
   * Authorization code flow to be used across all HTTP servlet requests or {@code null} before
   * initialized in {@link #initializeFlow()}.
   */
  private AuthorizationCodeFlow flow;

  @Override
  protected final void doGet(HttpServletRequest req, HttpServletResponse resp)
      throws ServletException, IOException {
    StringBuffer buf = req.getRequestURL();
    if (req.getQueryString() != null) {
      buf.append('?').append(req.getQueryString());
    }
    AuthorizationCodeResponseUrl responseUrl = new AuthorizationCodeResponseUrl(buf.toString());
    String code = responseUrl.getCode();
    if (responseUrl.getError() != null) {
      onError(req, resp, responseUrl);
    } else if (code == null) {
      resp.setStatus(HttpServletResponse.SC_BAD_REQUEST);
      resp.getWriter().print("Missing authorization code");
    } else {
      lock.lock();
      try {
        if (flow == null) {
          flow = initializeFlow();
        }
        String redirectUri = getRedirectUri(req);
        TokenResponse response = flow.newTokenRequest(code).setRedirectUri(redirectUri).execute();
        String userId = getUserId(req);
        Credential credential = flow.createAndStoreCredential(response, userId);
        onSuccess(req, resp, credential);
      } finally {
        lock.unlock();
      }
    }
  }

Any additional information below

If there are lots of requests, the Threads are blocked and waiting for the lock from other threads.
What the purpose of the lock? Is it onlyuseful for initializeFlow()?
Is the rest of the process thread safe?

Can we put

 String redirectUri = getRedirectUri(req);
        TokenResponse response = flow.newTokenRequest(code).setRedirectUri(redirectUri).execute();
        String userId = getUserId(req);
        Credential credential = flow.createAndStoreCredential(response, userId);
        onSuccess(req, resp, credential);

outside the lock?

Thanks
Regards,
Sylvain

[lqiu96]

Hey, apologies for the super delayed response. We are catching up on older issues. Is this still an issue on your end?

If so, can we get more information regarding this? Is this just a sub-optimal experience like waiting longer than expected or are the requests experiencing deadlock? Do you have a reproducer that can showcase what you are seeing?

This is an older library and as referenced our README we are only going to be fixing the critical issues:

This client library is supported but in maintenance mode only. We are fixing necessary bugs and adding essential features to ensure this library continues to meet your needs for accessing Google APIs. Non-critical issues will be closed. Any issue may be reopened if it is causing ongoing problems.