Skip to content

ConcurrentModificationException on load tests #766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dsukhoroslov opened this issue Jun 6, 2022 · 4 comments
Closed

ConcurrentModificationException on load tests #766

dsukhoroslov opened this issue Jun 6, 2022 · 4 comments
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@dsukhoroslov
Copy link

dsukhoroslov commented Jun 6, 2022

Describe the bug
We develop a custom Auth Server, it is based on the spring-authorization-server project, v.0.3.0. In load tests after running 20+ concurrent login sessions I see the following stack trace time to time:

2022-06-06 13:20:47.905 DEBUG 1 --- [http-nio-9000-exec-2201] o.s.security.web.FilterChainProxy        : Securing POST /oauth2/token
2022-06-06 13:20:47.906 DEBUG 1 --- [http-nio-9000-exec-2201] w.c.HttpSessionSecurityContextRepository : Retrieved SecurityContextImpl [Authentication=UsernamePasswordAuthenticationToken [Principal=13158530-f056-4cab-9c68-b7aacfb540d1, Credentials=[PROTECTED], Authenticated=true, Details=WebAuthenticationDetails [RemoteIpAddress=10.0.6.71, SessionId=261715E49D3BB35622225C6FFE58EED0], Granted Authorities=[SCOPE_openid, SCOPE_profile, SCOPE_email]]]
2022-06-06 13:20:47.906 DEBUG 1 --- [http-nio-9000-exec-2201] s.s.w.c.SecurityContextPersistenceFilter : Set SecurityContextHolder to SecurityContextImpl [Authentication=UsernamePasswordAuthenticationToken [Principal=13158530-f056-4cab-9c68-b7aacfb540d1, Credentials=[PROTECTED], Authenticated=true, Details=WebAuthenticationDetails [RemoteIpAddress=10.0.6.71, SessionId=261715E49D3BB35622225C6FFE58EED0], Granted Authorities=[SCOPE_openid, SCOPE_profile, SCOPE_email]]]
...
2022-06-06 13:20:47.953 DEBUG 1 --- [http-nio-9000-exec-2201] s.s.w.c.SecurityContextPersistenceFilter : Cleared SecurityContextHolder to complete request
2022-06-06 13:20:47.954 ERROR 1 --- [http-nio-9000-exec-2201] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception

java.util.ConcurrentModificationException: null
	at java.base/java.util.LinkedHashMap$LinkedHashIterator.nextNode(Unknown Source)
	at java.base/java.util.LinkedHashMap$LinkedValueIterator.next(Unknown Source)
	at org.springframework.security.oauth2.server.authorization.InMemoryOAuth2AuthorizationService.findByToken(InMemoryOAuth2AuthorizationService.java:138)
	at org.springframework.security.oauth2.server.authorization.authentication.CodeVerifierAuthenticator.authenticate(CodeVerifierAuthenticator.java:77)
	at org.springframework.security.oauth2.server.authorization.authentication.CodeVerifierAuthenticator.authenticateIfAvailable(CodeVerifierAuthenticator.java:66)
	at org.springframework.security.oauth2.server.authorization.authentication.ClientSecretAuthenticationProvider.authenticate(ClientSecretAuthenticationProvider.java:111)
	at org.springframework.security.authentication.ProviderManager.authenticate(ProviderManager.java:182)

The exception occurs in the InMemoryOAuth2AuthorizationService class, so it should be reproduceable on a clean spring-authorization-server instance, probably.

Thanks, Denis

@dsukhoroslov dsukhoroslov added the type: bug A general bug label Jun 6, 2022
@sjohnr
Copy link
Member

sjohnr commented Jun 6, 2022

@dsukhoroslov, thanks for the report! Please note:

As this implementation of OAuth2AuthorizationService is for getting started and not for production use, I don't believe we would need to pursue a fix. You are recommended to switch to JdbcOAuth2AuthorizationService. You can configure one by registering an @Bean (see docs), as demonstrated in the default sample. With that in mind, I'm going to close this for now. If there's something you think I've missed, please feel free to add additional comments and we can re-open if necessary.

@sjohnr sjohnr closed this as completed Jun 6, 2022
@sjohnr sjohnr self-assigned this Jun 6, 2022
@sjohnr sjohnr added status: invalid An issue that we don't feel is valid and removed type: bug A general bug labels Jun 6, 2022
@dsukhoroslov
Copy link
Author

dsukhoroslov commented Jun 6, 2022

Thanks @sjohnr, I see now. But JdbcOAuth2AuthorizationServer is not an option in my case, as I need a lightweight in-memory solution (my Auth Server is just a bridge between existing IAM platform and external service which does the real authentication using protocols other than OAuth2/OIDC) and has no persistence layer.

@sjohnr
Copy link
Member

sjohnr commented Jun 7, 2022

Gotcha, that is definitely a valid and interesting case! A couple of resources for you:

  • Support for NoSql datastores (Redis) #558 looks to add support for other (in-memory) persistence technologies like Redis which may be ideal for your situation. Work has started but it may be a bit before we can circle back to finishing it.
  • The How-to guide for implementing core services with JPA may give you a few pointers for building your own in the mean-time.
  • Because the OAuth2AuthorizationService is intended to be implemented by the consuming application, you can also copy the InMemoryOAuth2AuthorizationService and modify it to suit your needs. For example, perhaps the MaxSizeHashMap could be adapted to use a concurrent collection. If you find something that works well, feel free to share it and perhaps we could incorporate those changes into the project with a PR.

@dsukhoroslov
Copy link
Author

Yes, so far I increased MaxSizeHashMap size to 256 and added a small fix to findByToken method:

    public OAuth2Authorization findByToken(String token, @Nullable OAuth2TokenType tokenType) {
        Assert.hasText(token, "token cannot be empty");
        for (OAuth2Authorization authorization : this.authorizations.values()) {
            if (hasToken(authorization, token, tokenType)) {
                return authorization;
            }
        }
        List<OAuth2Authorization> values = new ArrayList<>(this.initializedAuthorizations.values());
        for (OAuth2Authorization authorization : values) {
            if (hasToken(authorization, token, tokenType)) {
                return authorization;
            }
        }
        return null;
    }

this solves the issue with ConcurrentModificationException for now, so testing it further..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

2 participants