Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

Highload bug #67

Closed
co11ter opened this issue Oct 25, 2016 · 11 comments
Closed

Highload bug #67

co11ter opened this issue Oct 25, 2016 · 11 comments

Comments

@co11ter
Copy link
Contributor

co11ter commented Oct 25, 2016

Hi!

I write the code:

type info struct {
    c http.ResponseWriter
}

type Test struct {
    *utron.BaseController
}

func (t *Test) Index() {
    fmt.Println(info{t.Ctx.Response()})
    t.Ctx.Redirect("http://123123123.12", http.StatusFound)
}

func NewTest() *Test {
    return &Test{}
}

send many concurent requests to /test:

siege -f urls.txt -c 10 -t30s -i

and have error:

{0xc420691ee0}
{0xc4207ca000}
{0xc4207ca0d0}
{0xc4207ca1a0}
{0xc4207ca270}
{0xc4207ca340}
{0xc4206a4d00}
{0xc4206a4d00}
fatal error: concurrent map writes

goroutine 794 [running]:
runtime.throw(0x9ef0e2, 0x15)
    /usr/lib/go/src/runtime/panic.go:566 +0x95 fp=0xc420505220 sp=0xc420505200
runtime.mapassign1(0x98f7a0, 0xc4207b8c00, 0xc420505330, 0xc420505340)
    /usr/lib/go/src/runtime/hashmap.go:458 +0x8ef fp=0xc420505308 sp=0xc420505220
net/textproto.MIMEHeader.Set(0xc4207b8c00, 0x9e551b, 0x8, 0x9edaf9, 0x13)
    /usr/lib/go/src/net/textproto/header.go:22 +0xca fp=0xc420505368 sp=0xc420505308
net/http.Header.Set(0xc4207b8c00, 0x9e551b, 0x8, 0x9edaf9, 0x13)
    /usr/lib/go/src/net/http/header.go:31 +0x53 fp=0xc4205053a0 sp=0xc420505368
net/http.Redirect(0xe63320, 0xc4206a4d00, 0xc4207b3680, 0x9edaf9, 0x13, 0x12e)
    /usr/lib/go/src/net/http/server.go:1819 +0xd7 fp=0xc420505488 sp=0xc4205053a0
github.com/gernest/utron.(*Context).Redirect(0xc420174e70, 0x9edaf9, 0x13, 0x12e)
    /home/coder/go/src/github.com/gernest/utron/context.go:189 +0x5f fp=0xc4205054c8 sp=0xc420505488

...

Two parallel routines get the same context. Maybe action signature should be

func (c *SomeController) Action(ctx *utron.Context) {}

instead to save the context into the controller??

@gernest
Copy link
Owner

gernest commented Oct 25, 2016

@co11ter Thanks for pointing this out.

I indeed the design had some issues. I'm open to suggestion, I will have to rewrite the implementation and possible change the API if possible.

Any input is welcome. You can help if you don't mind.

@gernest
Copy link
Owner

gernest commented Oct 25, 2016

Someone also pointed this out #57

@gernest
Copy link
Owner

gernest commented Oct 25, 2016

Problems seems to arise here https://github.com/gernest/utron/blob/master/routes.go#L313-L352

I think I might have ideas on how to fix this without changing of the API

@co11ter
Copy link
Contributor Author

co11ter commented Oct 25, 2016

I fix it for me like https://github.com/co11ter/utron/blob/master/routes.go#L382-L398, but I don't think that it's "true way". Maybe we have to create new controller per request?

@co11ter
Copy link
Contributor Author

co11ter commented Oct 25, 2016

Another fix co11ter@fb99f6f
but it change API:

type SomeController struct {
    *utron.BaseController
}

func NewSomeController() utron.Controller {
     return &SomeController{&utron.BaseController{}}
}

utron.RegisterController(NewSomeController)

I can send pull request if you want.

@gernest
Copy link
Owner

gernest commented Oct 25, 2016

@co11ter cool. Did you test the first one?

I don't think changing the API will be wise for now! Please send the PR, then we can maybe discuss there if any changes need to be made!

@gernest
Copy link
Owner

gernest commented Oct 25, 2016

I cant spopt the difference , please Open the PR so I can check the diff!

@gernest
Copy link
Owner

gernest commented Oct 25, 2016

I think, for now the best way will be to create a new Controller on each request.

@gernest
Copy link
Owner

gernest commented Oct 25, 2016

@co11ter please try to tun the example again to see if this bug is gone for good

@co11ter
Copy link
Contributor Author

co11ter commented Oct 27, 2016

yes, it fixed!

@gernest
Copy link
Owner

gernest commented Oct 27, 2016

@co11ter awesome. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants