Skip to content
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

Creating multiple https requests fail because of race condition #20228

Closed
markvandertol opened this issue Apr 4, 2016 · 5 comments
Closed

Creating multiple https requests fail because of race condition #20228

markvandertol opened this issue Apr 4, 2016 · 5 comments

Comments

@markvandertol
Copy link
Contributor

I tried to use akka-http to open multiple https connections at once, but most of the time this fails with an exception. Also, when I insert a delay between the requests it will always succeed.

I used akka-http-core, version 2.4.3 on Java Hotspot 1.8.0_77 (64-bits).

Sample code to reproduce the issue:

import akka.actor.ActorSystem
import akka.http.scaladsl.Http
import akka.http.scaladsl.model.{ HttpRequest, Uri}
import akka.stream.ActorMaterializer

implicit val system: ActorSystem = ActorSystem("my-system")
implicit val materializer = ActorMaterializer()
implicit val ec = system.dispatcher
val http = Http()
val request = HttpRequest(uri = Uri("https://www.google.com"))
http.singleRequest(request).onComplete(r => println(r))
http.singleRequest(request).onComplete(r => println(r))

The Futures don't complete and the following errors are logged:

[ERROR] [04/04/2016 21:50:43.770] [my-system-akka.actor.default-dispatcher-2] [akka://my-system/user/StreamSupervisor-1/flow-1-2-SlotProcessorInternalConnectionFlow] Duplicated server name of type 0
akka.actor.ActorInitializationException: akka://my-system/user/StreamSupervisor-1/flow-1-2-SlotProcessorInternalConnectionFlow: exception during creation
    at akka.actor.ActorInitializationException$.apply(Actor.scala:174)
    at akka.actor.ActorCell.create(ActorCell.scala:606)
    at akka.actor.ActorCell.invokeAll$1(ActorCell.scala:461)
    at akka.actor.ActorCell.systemInvoke(ActorCell.scala:483)
    at akka.dispatch.Mailbox.processAllSystemMessages(Mailbox.scala:282)
    at akka.dispatch.Mailbox.run(Mailbox.scala:223)
    at akka.dispatch.Mailbox.exec(Mailbox.scala:234)
    at scala.concurrent.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)
    at scala.concurrent.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)
    at scala.concurrent.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)
    at scala.concurrent.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)
Caused by: java.lang.IllegalArgumentException: Duplicated server name of type 0
    at javax.net.ssl.SSLParameters.setServerNames(SSLParameters.java:302)
    at sun.security.ssl.SSLEngineImpl.getSSLParameters(SSLEngineImpl.java:2103)
    at sun.security.ssl.SSLAlgorithmConstraints.<init>(SSLAlgorithmConstraints.java:91)
    at sun.security.ssl.Handshaker.init(Handshaker.java:264)
    at sun.security.ssl.Handshaker.<init>(Handshaker.java:227)
    at sun.security.ssl.ClientHandshaker.<init>(ClientHandshaker.java:177)
    at sun.security.ssl.SSLEngineImpl.initHandshaker(SSLEngineImpl.java:487)
    at sun.security.ssl.SSLEngineImpl.kickstartHandshake(SSLEngineImpl.java:683)
    at sun.security.ssl.SSLEngineImpl.beginHandshake(SSLEngineImpl.java:756)
    at akka.stream.impl.io.TLSActor.applySessionParameters(TLSActor.scala:185)
    at akka.stream.impl.io.TLSActor.<init>(TLSActor.scala:169)
    at akka.stream.impl.io.TLSActor$$anonfun$props$1.apply(TLSActor.scala:35)
    at akka.stream.impl.io.TLSActor$$anonfun$props$1.apply(TLSActor.scala:35)
    at akka.actor.TypedCreatorFunctionConsumer.produce(IndirectActorProducer.scala:87)
    at akka.actor.Props.newActor(Props.scala:213)
    at akka.actor.ActorCell.newActor(ActorCell.scala:562)
    at akka.actor.ActorCell.create(ActorCell.scala:588)
    ... 9 more

[ERROR] [04/04/2016 21:50:43.771] [my-system-akka.actor.default-dispatcher-2] [akka://my-system/user/StreamSupervisor-1/flow-2-2-SlotProcessorInternalConnectionFlow] Duplicated server name of type 0
akka.actor.ActorInitializationException: akka://my-system/user/StreamSupervisor-1/flow-2-2-SlotProcessorInternalConnectionFlow: exception during creation
[repeat of exact same stacktrace]

I did some debugging and I found it to be a race condition caused by modifying the same SSLParameters object from multiple threads at the same time. The mutable SSLParameters instance is created at the akka.http.scaladsl.DefaultSSLContextCreation#createDefaultClientHttpsContext method. This instance is then modified at akka.stream.impl.io.TLSActor#applySNI from two threads at once, leaving the serverNames property of the SSLParameters instance in an invalid state.

Internal details: The Oracle implementation of javax.net.ssl.SSLParameters#setServerNames first replaces the internal field with a new HashMap and then copies the server names passed as argument to that Map. Between the creation of the Map and the copying of the newly passed server names the first part of the race condition occurs and causes both threads to write to the same Map. Then the second part of the race condition occurs: while both threads write to the same Map, they both insert the same key, causing the Map to contains a duplicate key. This leaves the Map of the SSLParameters in an invalid state. A later call to sslParameters1.setServerNames(sslParameters2.getServerNames()) causes the exception shown above to be thrown.

A fix would be copy the SSLParameters before modifying them or use some sort of synchronization mechanism. Since SSLParameters just has some getter and setter methods, I would suggest copying it.

@ktoso
Copy link
Member

ktoso commented Apr 5, 2016

Ouch, thanks for finding this nasty bug!
We'll want to fix this before 2.4.4, if you have time to hakk on it let me know - you've did quite some great research already!

@ktoso ktoso added this to the 2.4.4 milestone Apr 5, 2016
@ktoso ktoso added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted 2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding bug t:http t:http:client labels Apr 5, 2016
@markvandertol
Copy link
Contributor Author

I shall have a shot at fixing this bug. The fix shouldn't be too much work.

I will go for copying the SSLParameters, since modifying them allows for a race condition when connecting to multiple domains.

@patriknw
Copy link
Member

patriknw commented Apr 5, 2016

That would be great, @markvandertol
Thanks

@ktoso ktoso added 3 - in progress Someone is working on this ticket and removed 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted labels Apr 5, 2016
@ktoso ktoso removed 2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding 3 - in progress Someone is working on this ticket labels Apr 6, 2016
@ktoso ktoso closed this as completed Apr 6, 2016
@ktoso ktoso modified the milestones: 2.4.4, 2.4.5 Apr 15, 2016
@jrudolph
Copy link
Member

Cleaning up things in TLSActor I came across this issue. I need a bit of clarification to prevent that I undo your fix in the process.

This instance is then modified at akka.stream.impl.io.TLSActor#applySNI from two threads at once, leaving the serverNames property of the SSLParameters instance in an invalid state.

Do I understand correctly: createDefaultClientHttpsContext creates only one mutable instance of SSLParameters and puts this instance into the firstSession = NegotiateNewSession() of potentially multiple instantiations of a TLS stage. This means that all those instantiations tried to use the same instance. This lead to the race condition when two TLSActors tried to use and mutate the same SSLParameters instance at the same time?

@markvandertol
Copy link
Contributor Author

That is correct. The TLSActor had to alter the SSLContext to apply the SNI settings for that specific connection, but since this SSLParameters instance was shared between multiple actors a race condition occurred. Two actors indeed altered the same SSLParameters instance at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants