[MLton] Int.fmt thread unsafe? (YUCK)

[Stephen Weeks]

For those who haven't read the Int.fmt code, the problem with thread
unsafety is a statically allocated buffer that is shared by all calls
to Int.fmt.  Off list, Henry suggested a way to use a statically
allocated buffer in the common case, while preserving thread safety.
The idea is to have a flag that keeps track of whether an instance of
Int.fmt is running.  If not, then use the static buffer; if so, then
dynamically allocate a buffer.

This seems like a generally useful trick, so I've attempted to express
the idea in the structure below (I'm open to better names than "One",
perhaps "OneAtATime"?).  In the case of Int.fmt, the code would be
changed to:

  val one = One.make (fn () => CharArray.array (maxNumDigits, #"\000"))

  fun fmt radix (n: int): string =
     One.use
     (one, fn buf =>
      ... the old fmt code that uses buf ...)

First, does the code look correct (and thread safe)?

Second, in the particular case of Int.fmt, how does this approach
compare to wrapping the whole body of Int.fmt in a Thread.atomically
and always using the statically allocated buffer?  I see that it makes
the critical section smaller, which is good.  But it does add two
tests that wouldn't otherwise be there.  Neither of those observations
sway me as to which approach is better, since Int.fmt seems fast
enough to put the whole thing in a critical section and a couple of
tests one way or the other will be dominated by the cost of the
Int.fmt code.

Thoughts?

----------------------------------------------------------------------
structure One:
   sig
      type 'a t

      val make: (unit -> 'a) -> 'a t
      val use: 'a t * ('a -> 'b) -> 'b
   end =
   struct
      datatype 'a t = T of {more: unit -> 'a,
                            static: 'a,
                            staticIsInUse: bool ref}

      fun make f = T {more = f,
                      static = f (),
                      staticIsInUse = ref false}

      fun use (T {more, static, staticIsInUse}, f) =
         let
            val () = Primitive.Thread.atomicBegin ()
            val b = !staticIsInUse
            val () = if b then () else staticIsInUse := true
            val () = Primitive.Thread.atomicEnd ()
         in
            DynamicWind.wind
            (fn () => f (if b then more () else static),
             fn () => if b then () else staticIsInUse := false)
         end
   end