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

reading from assets.Files not concurrency safe, or repeated use safe #10

Open
rbtcollins opened this issue Sep 17, 2019 · 0 comments
Open

Comments

@rbtcollins
Copy link

rbtcollins commented Sep 17, 2019

I followed the gin example code that used go-asset-builder, which looks roughly like the below; I then had mysterious test failures with empty result bodies until I put in the panic that is in the below code (and not in the example I cribbed from :)).

The bug is fairly straight forward - the Files instance is already 'open' so the buffers are never recreated and everything can only be read once. It also humorously (or not) means its not concurrency safe - multithreaded reads can interleave different parts of the file and move the buffer pointer forward.

	t := template.New("")
	for name, file := range assets.Files {
		if file.IsDir() || !strings.HasSuffix(name, ".gohtml") {
			continue
		}
		h, err := ioutil.ReadAll(file)
		if err != nil {
			return nil, err
		}
		if len(h) == 0 {
			panic("zero length template")
		}
		t, err = t.New(name).Parse(string(h))
		if err != nil {
			return nil, err
		}
	}

With that assert, my tests rather than failing with empty bodies fail when instantiating my router:

--- FAIL: TestServerTestSuite (0.00s)
    --- FAIL: TestServerTestSuite/TestRootDocHTML (0.00s)
        .../suite.go:61: test panicked: zero length template
    --- FAIL: TestServerTestSuite/TestRootDocJSON (0.00s)
        .../suite.go:61: test panicked: zero length template
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