Skip to content

Commit

Permalink
Replace frightening fromBuilder method with new fromCollection
Browse files Browse the repository at this point in the history
The existing fromBuilder method reused the same builder for multiple `invert`
calls, which is not safe under multithreading: one thread might, for example,
clear the builder while another thread is adding elements.

This patch changes the implementation to get a new builder every time from the
CanBuildFrom, and puts that behind the existing fromCollection interface. The
existing injections for specific collections should not change behaviour.
  • Loading branch information
bkirwi-stripe committed Jul 9, 2015
1 parent bc964c9 commit 3f1cfb7
Showing 1 changed file with 7 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,15 @@ object JsonNodeInjection extends LowPriorityJson with java.io.Serializable {
}

// This causes diverging implicits
def collectionJson[T, C <: Traversable[T]](implicit cbf: CanBuildFrom[Nothing, T, C],
jbij: JsonNodeInjection[T]): JsonNodeInjection[C] = fromBuilder(cbf())

def fromBuilder[T, C <: Traversable[T]](builder: Builder[T, C])(implicit jbij: JsonNodeInjection[T]): JsonNodeInjection[C] =
def collectionJson[T, C <: Traversable[T]](implicit cbf: CanBuildFrom[Nothing, T, C], jbij: JsonNodeInjection[T]): JsonNodeInjection[C] =
new AbstractJsonNodeInjection[C] {
def apply(l: C) = {
val ary = JsonNodeFactory.instance.arrayNode
l foreach { t => ary.add(jbij(t)) }
ary
}
override def invert(n: JsonNode): Try[C] = {
builder.clear
val builder = cbf()
var inCount = 0
n.getElements.asScala.foreach { jn =>
inCount += 1
Expand All @@ -154,19 +151,19 @@ object JsonNodeInjection extends LowPriorityJson with java.io.Serializable {
}

implicit def listJson[T: JsonNodeInjection]: JsonNodeInjection[List[T]] =
fromBuilder(List.newBuilder[T])
collectionJson[T, List[T]]

implicit def vectorJson[T: JsonNodeInjection]: JsonNodeInjection[Vector[T]] =
fromBuilder(Vector.newBuilder[T])
collectionJson[T, Vector[T]]

implicit def indexedSeqJson[T: JsonNodeInjection]: JsonNodeInjection[IndexedSeq[T]] =
fromBuilder(IndexedSeq.newBuilder[T])
collectionJson[T, IndexedSeq[T]]

implicit def seqJson[T: JsonNodeInjection]: JsonNodeInjection[Seq[T]] =
fromBuilder(Seq.newBuilder[T])
collectionJson[T, Seq[T]]

implicit def setJson[T: JsonNodeInjection]: JsonNodeInjection[Set[T]] =
fromBuilder(Set.newBuilder[T])
collectionJson[T, Set[T]]

implicit def mapJson[V: JsonNodeInjection]: JsonNodeInjection[Map[String, V]] =
new AbstractJsonNodeInjection[Map[String, V]] {
Expand Down

0 comments on commit 3f1cfb7

Please sign in to comment.