Make string Injections safer. Fix #199

bijection-core/src/main/scala/com/twitter/bijection/AtomicSharedMutable.scala

@@ -0,0 +1,21 @@
+package com.twitter.bijection
+
+import java.util.concurrent.atomic.AtomicReference
+
+/**
+ * This class is for sharing an object between threads
+ *  when allocation of a new one is not that cheap, but
+ *  better than blocking.
+ */
+private[bijection] class AtomicSharedState[T <: AnyRef](cons: () => T) {
+  private[this] val ref = new AtomicReference[T](cons())
+
+  def get: T = {
+    val borrow = ref.getAndSet(null.asInstanceOf[T])
+    if (null == borrow) cons()
+    else borrow
+  }
+  def release(t: T): Unit = {
+    ref.lazySet(t)
+  }
+}

bijection-core/src/main/scala/com/twitter/bijection/StringInjections.scala

@@ -16,13 +16,13 @@ limitations under the License.
 
 package com.twitter.bijection
 
+import com.twitter.bijection.Inversion.attempt
 import java.net.{ URLDecoder, URLEncoder, URL }
+import java.nio.charset.{ Charset, CoderResult, CodingErrorAction }
+import java.nio.{ ByteBuffer, CharBuffer }
 import java.util.UUID
-
 import scala.annotation.tailrec
 import scala.collection.generic.CanBuildFrom
-
-import com.twitter.bijection.Inversion.attempt
 import scala.util.Try
 
 trait StringInjections extends NumericInjections {
@@ -32,9 +32,40 @@ trait StringInjections extends NumericInjections {
 
   def withEncoding(encoding: String): Injection[String, Array[Byte]] =
     new AbstractInjection[String, Array[Byte]] {
+      private[this] val decRef =
+        new AtomicSharedState({ () =>
+          val dec = Charset.forName(encoding)
+            .newDecoder
+            .onUnmappableCharacter(CodingErrorAction.REPORT)
+          val buf = CharBuffer.allocate(1024) // something big enough, if not big enough, allocate
+          (dec, buf)
+        })
+
       def apply(s: String) = s.getBytes(encoding)
       override def invert(b: Array[Byte]) =
-        attempt(b) { new String(_, encoding) }
+        // toString on ByteBuffer is nicer, so use it
+        attempt(ByteBuffer.wrap(b)) { bb =>
+          //these are mutable, so it can't be shared trivially
+          //avoid GC pressure and (probably) perform better
+          val decBuf = decRef.get
+          val dec = decBuf._1
+          val buf = decBuf._2
+          val maxSpace = (b.length * dec.maxCharsPerByte).toInt + 1
+          val thisBuf = if (maxSpace > buf.limit) CharBuffer.allocate(maxSpace) else buf
+
+          // this is the error free result
+          @inline def assertUnderFlow(cr: CoderResult) { if(!cr.isUnderflow) cr.throwException }
+          assertUnderFlow(dec.reset.decode(bb, thisBuf, true))
+          assertUnderFlow(dec.flush(thisBuf))
+          // set the limit to be the position
+          thisBuf.flip
+          val str = thisBuf.toString
+          // make sure the buffer we store is clear.
+          buf.clear
+          // we don't reset with the larger buffer to avoid memory leaks
+          decRef.release(decBuf)
+          str
+        }
     }
 
   // Some bijections with string from standard java/scala classes:

bijection-core/src/test/scala/com/twitter/bijection/StringBijectionLaws.scala

@@ -38,7 +38,7 @@ object StringArbs extends BaseProperties {
 class StringBijectionLaws extends CheckProperties with BaseProperties {
 
   property("round trips string -> Array[String]") {
-    isLooseInjection[String, Array[Byte]]
+    isInjection[String, Array[Byte]]
   }
 
   implicit val symbol = arbitraryViaFn { (s: String) => Symbol(s) }