On Tue, Jun 21, 2016 at 5:56 AM, Ian Lance Taylor <i...@golang.org> wrote:

> On Mon, Jun 20, 2016 at 5:57 PM, Bryan Reynaert <alke...@gmail.com> wrote:
> >
> > the following code panics with: runtime error: slice bounds out of range
> >
> >
> > var testTransform = transform.Chain(norm.NFD, norm.NFC)
> >
> > func main() {
> > for i := 0; i < 200; i++ {
> > go func() {
> > transform.String(testTransform, "nonemptystring")
> > }()
> > }
> > time.Sleep(time.Second)
> > }
> >
> > The reason is transform.Chain returns a Transformer which keeps an
> internal
> > state
> > while sequentially applying each transformation. When called by two
> > goroutines,
> > this state gets corrupted.
> >
> > For me, this was unexpected, as each transform by itself seems
> thread-safe.
> > Errors
> > caused by this behaviour might be hard to track. I suggest changing the
> > implementation
> > of transform.Chain.Transform to generate a new state for each call or
> make a
> > note about
> > this in the docs.
> >
> > Or maybe I was expecting something I should not?
> >
> > I can write a thread-safe version of chain.Transform if this is the way
> to
> > go. Please comment.
>
> The general rule for the standard library is that if a type does not
> explicitly say that it is safe to call methods from multiple
> goroutines, then it is not safe to do so.  That said, it may be
> reasonable to explicitly state that in this case.
>
> I don't think it should be made thread-safe unless the result is just
> as efficient.  Efficiency matters here.
>
Indeed. The reason this is not thread-safe is especially for performance
reasons. Also, Transformers are, in general, not thread-safe (which is
already implied by the Reset method).

Making Chain thread-safe even wouldn't even work for a sequence of
thread-safe Transformers without sacrificing performance, as you would have
to allocate largish buffers on every call to Transform.

The solution is really not to share Transformer returned by Chain. On the
bright side, sort of, the creation of the buffers (the state you want to
replicate on each call to transform) is biggest cost of creating a new
Transformer using Chain. So if allocating the buffers is not an issue, you
can do

var testTransform = func() Transformer { return transform.Chain(norm.NFD,
norm.NFC) }

and

transform.String(testTransform(), "nonemptystring")

Alternatively, if you are dealing with small strings, you could avoid using
Chain altogether:

norm.NFC.String(norm.NFD.String("nonemptystring")).





>
> Ian
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to