Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fatal error: concurrent map read and map write #3699

Closed
zhuzaiye opened this issue Aug 15, 2023 · 5 comments
Closed

fatal error: concurrent map read and map write #3699

zhuzaiye opened this issue Aug 15, 2023 · 5 comments

Comments

@zhuzaiye
Copy link

Description

fatal error: concurrent map read and map write

goroutine 4030 [running]:
github.com/gin-gonic/gin/binding.setByForm({0xf41cd0, 0x7703a18, 0x18e}, {{0xf6336c, 0xe}, {0x0, 0x0}, {0x14d4ad4, 0xf41cd0}, {0xf6337b, ...}, ...}, ...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:168 +0x34
github.com/gin-gonic/gin/binding.formSource.TrySet(0x7e78080, {0xf41cd0, 0x7703a18, 0x18e}, {{0xf6336c, 0xe}, {0x0, 0x0}, {0x14d4ad4, 0xf41cd0}, ...}, ...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:73 +0x68
github.com/gin-gonic/gin/binding.tryToSetValue({0xf41cd0, 0x7703a18, 0x18e}, {{0xf6336c, 0xe}, {0x0, 0x0}, {0x14d4ad4, 0xf41cd0}, {0xf6337b, ...}, ...}, ...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:164 +0x200
github.com/gin-gonic/gin/binding.mapping({0xf41cd0, 0x7703a18, 0x18e}, {{0xf6336c, 0xe}, {0x0, 0x0}, {0x14d4ad4, 0xf41cd0}, {0xf6337b, ...}, ...}, ...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:106 +0xec
github.com/gin-gonic/gin/binding.mapping({0x101f760, 0x77039e0, 0x199}, {{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x0, ...}, ...}, ...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:124 +0x46c
github.com/gin-gonic/gin/binding.mapping({0xee59d0, 0x77039e0, 0x16}, {{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x0, ...}, ...}, ...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:95 +0x2ec
github.com/gin-gonic/gin/binding.mappingByPtr({0xee59d0, 0x77039e0}, {0x14cb214, 0x7e78080}, {0x10886a7, 0x4})
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:77 +0xf8
github.com/gin-gonic/gin/binding.mapFormByTag({0xee59d0, 0x77039e0}, 0x7e78080, {0x10886a7, 0x4})
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:59 +0x238
github.com/gin-gonic/gin/binding.mapForm(...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:34
github.com/gin-gonic/gin/binding.formBinding.Bind({}, 0x7a5ea00, {0xee59d0, 0x77039e0})
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/binding/form.go:29 +0xc4
github.com/gin-gonic/gin.(*Context).ShouldBindWith(...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:741
github.com/gin-gonic/gin.(*Context).ShouldBind(0x6c18870, {0xee59d0, 0x77039e0})
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:696 +0x2b4
code.iflytek.com/EBG_WB_Osaka/elstStudentApi/pkg/app.(*core).ShouldBind(0x80d79f8, {0xee59d0, 0x77039e0})
D:/go/src/EBG_WB_Osaka/elstStudentApi/pkg/app/response.go:75 +0x74
code.iflytek.com/EBG_WB_Osaka/elstStudentApi/controllers/studentapi.(*eikenSpeaking).PostEikenExaminationLog(0x1e0e5d0, 0x6c18870)
D:/go/src/EBG_WB_Osaka/elstStudentApi/controllers/studentapi/eiken.go:189 +0x60
github.com/gin-gonic/gin.(*Context).Next(...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
code.iflytek.com/EBG_WB_Osaka/elstStudentApi/routers.checkStudentToken.func1(0x6c18870)
D:/go/src/EBG_WB_Osaka/elstStudentApi/routers/check_token.go:18 +0x80
github.com/gin-gonic/gin.(*Context).Next(...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
code.iflytek.com/EBG_WB_Osaka/elstStudentApi/routers.CorsDomain.func1(0x6c18870)
D:/go/src/EBG_WB_Osaka/elstStudentApi/routers/cors_domain.go:19 +0x1b4
github.com/gin-gonic/gin.(*Context).Next(...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
github.com/gin-gonic/gin.CustomRecoveryWithWriter.func1(0x6c18870)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/recovery.go:102 +0x94
github.com/gin-gonic/gin.(*Context).Next(...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
github.com/gin-gonic/gin.LoggerWithConfig.func1(0x6c18870)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/logger.go:240 +0x9c
github.com/gin-gonic/gin.(*Context).Next(...)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0x3d10b60, 0x6c18870)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/gin.go:620 +0x554
github.com/gin-gonic/gin.(*Engine).ServeHTTP(0x3d10b60, {0x14ce2a4, 0x7e1efa0}, 0x7a5ea00)
C:/Users/yangwu11/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/gin.go:576 +0x1f8
net/http.serverHandler.ServeHTTP({0x46c2280}, {0x14ce2a4, 0x7e1efa0}, 0x7a5ea00)
D:/Program Files/Go/src/net/http/server.go:2947 +0x2f0
net/http.(*conn).serve(0x63d78c0, {0x14cf058, 0x58a1e90})
D:/Program Files/Go/src/net/http/server.go:1991 +0x654
created by net/http.(*Server).Serve
D:/Program Files/Go/src/net/http/server.go:3102 +0x4e0

How to reproduce

1. form bind script:
var form FormEkExaminationLog
if err := c.ShouldBind(&form); err != nil {
    g.Error(err.Error())
    return
}

2. self define the bind func
func (g *core) ShouldBind(data interface{}) (err error) {
	if g.ctx.ContentType() == "application/json" {
		return g.ctx.ShouldBindBodyWith(data, binding.JSON)
	}
	return g.ctx.ShouldBind(data)
}

Expectations

When press test, the fatal error coccur.
Why?

func (c *Context) ShouldBind(obj any) error {
	b := binding.Default(c.Request.Method, c.ContentType())
	return c.ShouldBindWith(obj, b)
}

the gin built map is not safe concurrent?

Environment

  • go version: 1.18
  • gin version (or commit ref): 1.9.1
  • operating system:
@zhuzaiye
Copy link
Author

func setByForm(value reflect.Value, field reflect.StructField, form map[string][]string, tagValue string, opt setOptions) (isSet bool, err error) {
	vs, ok := form[tagValue]
	if !ok && !opt.isDefaultExists {
		return false, nil
	}

	switch value.Kind() {
	case reflect.Slice:
		if !ok {
			vs = []string{opt.defaultValue}
		}
		return true, setSlice(vs, value, field)
	case reflect.Array:
		if !ok {
			vs = []string{opt.defaultValue}
		}
		if len(vs) != value.Len() {
			return false, fmt.Errorf("%q is not valid value for %s", vs, value.Type().String())
		}
		return true, setArray(vs, value, field)
	default:
		var val string
		if !ok {
			val = opt.defaultValue
		}

		if len(vs) > 0 {
			val = vs[0]
		}
		return true, setWithProperType(val, value, field)
	}
}

github.com/gin-gonic/gin@v1.9.1/binding/form_mapping.go:167
the setByForm func's input param form is map type, the map instance is unsafe.
The reason is here?

@zhuzaiye
Copy link
Author

zhuzaiye commented Aug 15, 2023

This is my one route:

api.POST("eiken/training-log", studentapi.EikenSpeakingController.PostEikenTrainingLog, integral.PushIntegralOfSelfExeWithResType12, integral.PushIntegralOfEikenSpeak)

The form shoud bind operation in the 1st handler function, the left function in the route chain will should bind to get the post data for working.
The shoud bind in the route chain will occur the bug "[fatal error: concurrent map read and map write]"

@zhuzaiye
Copy link
Author

zhuzaiye commented Aug 16, 2023

Using the single-factor method, determine the problem occurring in the handlesfunc chain.

Example:

api.POST("eiken/training-log", studentapi.EikenSpeakingController.PostEikenTrainingLog, integral.PushIntegralOfSelfExeWithResType12, integral.PushIntegralOfEikenSpeak)

-- PostEikenTrainingLog

var form FormEkExaminationLog
	if err := g.ShouldBind(&form); err != nil {
		g.Error(err.Error())
		return
	}

-- PushIntegralOfSelfExeWithResType12

func PushIntegralOfSelfExeWithResType12(c *gin.Context) {
	go func() {
		integralOfBaseExercise(c, modelOfSelfExercise, generalModeOfType12)
	}()
}

func integralOfBaseExercise(c *gin.Context, exerciseType, exerciseModule int) {
	studentId := v4.GetStudentIdByToken(c)
	if studentId == 0 {
		return
	}
	g := app.Gin(c)
	var form exerciseForm
	if err := g.ShouldBind(&form); err != nil {
		logging.Error(err.Error())
		return
	}

The error will not occur if PushIntegralOfSelfExeWithResType12 is not executed using the goroutine.

Currently, my solution is to either not use the goroutine method in the handlerfuncs chain, or to start it in a handlerfunc and use goroutine within a transaction.

PS: handlerfuncs chain+ shouldbind+goroutine will not trigger this error????

@zhuzaiye
Copy link
Author

image

using context is correct way!!!

@zhuzaiye
Copy link
Author

zhuzaiye commented Nov 9, 2023

The following is my guess reson and fix my bug solution.

sence:

router handler_chains: api.POST("test/goroutine-chain", v4.ContestController.SubmitResult, integral.NewPushIntegralOfWriteContest)

// first handled chain
func (s *contest) SubmitResult(c *gin.Context) {
	g := app.Gin{C: c}
	studentId := GetStudentIdByToken(c)
	var form constants.ScoreLog
	if err := g.ShouldBind(&form); err != nil {
		g.Error("form param_missing")
		logging.Error(err.Error())
		return
	}
}

// second handled chain
func NewPushIntegralOfWriteContest(c *gin.Context) {
	go func() {
		studentId := v4.GetStudentIdByToken(c)
		var form FormIntegral
                // second handler chain get the request data 
		if err := c.ShouldBind(&form); err != nil {
			logging.Error(err.Error())
			return
		}
                // Main business code
                ...     
        }
}

github.com/gin-gonic/gin@v1.7.7/context.go:705

func (c *Context) ShouldBindWith(obj interface{}, b binding.Binding) error {
	return b.Bind(c.Request, obj)
}

github.com/gin-gonic/gin@v1.7.7/binding/form.go:54

func (formMultipartBinding) Bind(req *http.Request, obj interface{}) error {
	if err := req.ParseMultipartForm(defaultMemory); err != nil {
		return err
	}
	if err := mappingByPtr(obj, (*multipartRequest)(req), "form"); err != nil {
		return err
	}

	return validate(obj)
}

go1.18/src/net/http/request.go:1299

func (r *Request) ParseMultipartForm(maxMemory int64) error {
	if r.MultipartForm == multipartByReader {
		return errors.New("http: multipart handled by MultipartReader")
	}
	var parseFormErr error
	if r.Form == nil {
		// Let errors in ParseForm fall through, and just
		// return it at the end.
		parseFormErr = r.ParseForm()
	}
	if r.MultipartForm != nil {
		return nil
	}

	mr, err := r.multipartReader(false)
	if err != nil {
		return err
	}

	f, err := mr.ReadForm(maxMemory)
	if err != nil {
		return err
	}

        // Point: 1 
	if r.PostForm == nil {
		r.PostForm = make(url.Values)
	}
        // The reason: fatal error: concurrent map writes
        // here will write the r.PostForm
        // the type of r.PostForm is map[string][]string in `go1.18/src/net/url/url.go:865`, which is unsafe concurrent map
	for k, v := range f.Value {
		r.Form[k] = append(r.Form[k], v...)
		// r.PostForm should also be populated. See Issue 9305.
		r.PostForm[k] = append(r.PostForm[k], v...)
	}

	r.MultipartForm = f

	return parseFormErr
}

go1.18/src/net/url/url.go:865

// no safe cocurrent 
type Values map[string][]string

To fix:

// second handled chain
func NewPushIntegralOfWriteContest(c *gin.Context) {
        studentId := v4.GetStudentIdByToken(c)
        var form FormIntegral
        // the scond bind the PostForm cannot be used in goroutine and must be executed sequentially
        if err := c.ShouldBind(&form); err != nil {
                logging.Error(err.Error())
                return
        }
	go func() {
                // Main business code
                ...     
        }
}

@zhuzaiye zhuzaiye closed this as completed Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant