search

wiremock / wiremock-state-extension

Adds support for transporting state across different API mock stubs
Apache License 2.0
16 stars 5 forks source link

ConcurrentModificationException in TransactionEventListener#afterComplete #119

Closed boris-senapt closed 8 months ago

boris-senapt commented 8 months ago

Proposal

I see this exception when running. It doesn't seem to cause problems

18:59:02.813 [qtp521311335-52] WARN  org.eclipse.jetty.server.HttpChannel {} - ...
java.util.ConcurrentModificationException: null
    at java.base/java.util.HashMap$KeySet.forEach(HashMap.java:1019) ~[?:?]
    at org.wiremock.extensions.state.extensions.TransactionEventListener.afterComplete(TransactionEventListener.java:56) ~[wiremock-state-extension-0.6.1.jar:?]
    at com.github.tomakehurst.wiremock.extension.ServeEventListener.onEvent(ServeEventListener.java:41) ~[wiremock-3.3.1.jar:?]
    at com.github.tomakehurst.wiremock.http.StubRequestHandler.lambda$triggerListeners$0(StubRequestHandler.java:138) ~[wiremock-3.3.1.jar:?]
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) ~[?:?]
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179) ~[?:?]
    at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133) ~[?:?]
    at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939) ~[?:?]
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
    at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) ~[?:?]
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) ~[?:?]
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
    at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[?:?]
    at com.github.tomakehurst.wiremock.http.StubRequestHandler.triggerListeners(StubRequestHandler.java:138) ~[wiremock-3.3.1.jar:?]
    at com.github.tomakehurst.wiremock.http.StubRequestHandler.afterResponseSent(StubRequestHandler.java:114) ~[wiremock-3.3.1.jar:?]
    at com.github.tomakehurst.wiremock.http.AbstractRequestHandler.handle(AbstractRequestHandler.java:104) ~[wiremock-3.3.1.jar:?]
    at com.github.tomakehurst.wiremock.servlet.WireMockHandlerDispatchingServlet.service(WireMockHandlerDispatchingServlet.java:157) ~[wiremock-3.3.1.jar:?]
    at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587) ~[jetty-jakarta-servlet-api-5.0.2.jar:?]
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:764) ~[jetty-servlet-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1665) ~[jetty-servlet-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:527) ~[jetty-servlet-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1381) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:484) ~[jetty-servlet-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1303) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:822) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:141) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.Server.handle(Server.java:563) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.HttpChannel$RequestDispatchable.dispatch(HttpChannel.java:1598) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:753) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:501) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:287) ~[jetty-server-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:314) ~[jetty-io-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100) ~[jetty-io-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53) ~[jetty-io-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969) ~[jetty-util-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194) ~[jetty-util-11.0.18.jar:11.0.18]
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149) ~[jetty-util-11.0.18.jar:11.0.18]
    at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]

It seems to be cause by 

var contextNames = transactionManager.getContextNamesByRequestId(requestId);
contextNames.forEach((contextName) -> transactionManager.deleteTransaction(requestId, contextName));

Where transactionManager.getContextNamesByRequestId returns the the keySet() of transaction:" and transactionManager.deleteTransaction accesses the same Map and calls remove()

Reproduction steps

Not entirely sure here - I haven't looked too much into what the TransactionManager does or why its used.

References

No response

dirkbolte commented 8 months ago

I wasn't able to reproduce it and am not really sure which scenario can cause this (the request ID should be unique, so the listener should only run once). Anyhow I think I have something which can prevent this. Can you give the code in the PR that I just opened a try?

boris-senapt commented 8 months ago

Will do.

I think the change makes sense anyway as from inside a synchonized method you were returning the contents of store (a non-thread safe HashMap); the behaviour of concurrent access would also be undefined.

Not related to this issue, but I think it's more robust.

boris-senapt commented 8 months ago

I ran the following

git clone git@github.com:wiremock/wiremock-state-extension.git
git sw bugfix/concurrent-transaction-modification
gradle publishToMavenLocal

This published 0.6.0-SNAPSHOT, so I updated my pom.xml with that version and ran my project test that exhibited the issue.

I no longer see the exception thrown.

dirkbolte commented 8 months ago

thx for checking.

dirkbolte commented 8 months ago

Version 0.6.2 containing this fix is available now.