May fail when parsing YAML many times in parallel ( with yaml.MapSlice )

[k1LoW]

Hi @goccy. I always use your packages conveniently. Thank you !!

When trying to unmarshal YAML by preparing the following struct, it always succeeds if it is done once.

type mappedSteps struct {
	Steps yaml.MapSlice `yaml:"steps,omitempty"`
}

However, it may fail when trying to unmarshal the same YAML many times in parallel.

Please run playground: https://go.dev/play/p/OpLrdNGRItg

If there is a workaround for this problem, please let me know.

[kortschak]

This is due to context pool contamination; contexts that are released into the pool are not properly cleared wrt the isLiteral field, leaving previous state to poison future use.

The relevant lines are here. Note that no action to clear the context is made

go-yaml/scanner/context.go

Lines 48 to 50 in 33858b4

	func (c *Context) release() {
	ctxPool.Put(c)
	}

and when the context is obtained, it is cleared with a call to *context.reset

go-yaml/scanner/context.go

Lines 42 to 46 in 33858b4

	func newContext(src []rune) *Context {
	ctx := ctxPool.Get().(*Context)
	ctx.reset(src)
	return ctx
	}

but *context.reset does not clear isLiteral

go-yaml/scanner/context.go

Lines 52 to 59 in 33858b4

	func (c *Context) reset(src []rune) {
	c.idx = 0
	c.size = len(src)
	c.src = src
	c.tokens = c.tokens[:0]
	c.resetBuffer()
	c.isSingleLine = true
	}

I would not be surprised if different YAML source would result in poisoning different flags, so just clearing the isLiteral field is probably not a safe option.

This is a fix that is future safe.

diff --git a/scanner/context.go b/scanner/context.go
index e629a5c..1f0ce1b 100644
--- a/scanner/context.go
+++ b/scanner/context.go
@@ -50,12 +50,13 @@ func (c *Context) release() {
 }
 
 func (c *Context) reset(src []rune) {
-       c.idx = 0
-       c.size = len(src)
-       c.src = src
-       c.tokens = c.tokens[:0]
-       c.resetBuffer()
-       c.isSingleLine = true
+       *c = Context{
+               buf:          c.buf[:0],
+               size:         len(src),
+               src:          src,
+               tokens:       c.tokens[:0],
+               isSingleLine: true,
+       }
 }
 
 func (c *Context) resetBuffer() {
diff --git a/yaml_test.go b/yaml_test.go
index 4e344c8..a691145 100644
--- a/yaml_test.go
+++ b/yaml_test.go
@@ -2,6 +2,7 @@ package yaml_test
 
 import (
        "bytes"
+       "fmt"
        "reflect"
        "testing"
 
@@ -582,3 +583,53 @@ baz:
                }
        })
 }
+
+const mapSliceYAML = `steps:
+  req0:
+    desc: Get /users/1
+    req:
+      /users/1:
+        get: nil
+    test: |
+      current.res.status == 200
+  req1:
+    desc: Get /private
+    req:
+      /private:
+        get: nil
+    test: |
+      current.res.status == 403
+  req2:
+    desc: Get /users
+    req:
+      /users:
+        get: nil
+    test: |
+      current.res.status == 200
+`
+
+type mappedSteps struct {
+       Steps yaml.MapSlice `yaml:"steps,omitempty"`
+}
+
+func TestParallel(t *testing.T) {
+       for i := 0; i < 100; i++ {
+               t.Run(fmt.Sprintf("i=%d", i), func(t *testing.T) {
+                       t.Parallel()
+                       for i := 0; i < 10; i++ {
+                               m := mappedSteps{
+                                       Steps: yaml.MapSlice{},
+                               }
+                               if err := yaml.Unmarshal([]byte(mapSliceYAML), &m); err != nil {
+                                       t.Fatal(err)
+                               }
+                               for _, s := range m.Steps {
+                                       _, ok := s.Value.(map[string]interface{})
+                                       if !ok {
+                                               t.Fatal("failed to parse")
+                                       }
+                               }
+                       }
+               })
+       }
+}

[goccy]

Thank you for your report !
I fixed this problem with #322