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

panic: runtime error: SIGSEGV on non existing directory #390

Closed
krhubert opened this issue Mar 27, 2017 · 2 comments · Fixed by #396
Closed

panic: runtime error: SIGSEGV on non existing directory #390

krhubert opened this issue Mar 27, 2017 · 2 comments · Fixed by #396
Assignees
Labels

Comments

@krhubert
Copy link

krhubert commented Mar 27, 2017

Hi,

There is panic (trying to execute nil function) when there is the directory non exists.

package main

import (
	"log"

	"go.uber.org/zap"
	"go.uber.org/zap/zapcore"
)

func main() {
	cfg := zap.NewProductionConfig()
	cfg.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
	cfg.Level.SetLevel(zapcore.InfoLevel)
	cfg.DisableCaller = true
	cfg.OutputPaths = []string{"/tmp/non_existing_dir/file.log"}
	logger, err := cfg.Build() // panic()
	if err != nil {
		log.Fatal(err)
	}
	logger.Info("test message", zap.String("foo", "bar"))
}

After run:

[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x605933]

goroutine 1 [running]:
go.uber.org/zap.Config.openSinks(0xc420011218, 0x100, 0xc420011230, 0x699642, 0x4, 0x699426, 0x3, 0x6998f4, 0x5, 0x699318, ...)
	/usr/src/go/src/go.uber.org/zap/config.go:220 +0x193
go.uber.org/zap.Config.Build(0xc420011218, 0x100, 0xc420011230, 0x699642, 0x4, 0x699426, 0x3, 0x6998f4, 0x5, 0x699318, ...)
	/usr/src/go/src/go.uber.org/zap/config.go:161 +0xb1
main.main()

The problem is in go.uber.org/zap/config.go#220 and 225-226 in openSinks method - there should be check for nil clouseOut/closeErr or zap.Open should return nopcloser function.

@aajtodd
Copy link

aajtodd commented Mar 28, 2017

Just ran into this as well when testing on file/directory without enough permission to create the log file.

akshayjshah pushed a commit that referenced this issue Mar 28, 2017
If the config struct includes invalid output paths, make sure that we don't try
to execute a nil function pointer.

Fixes #390.
@akshayjshah akshayjshah self-assigned this Mar 28, 2017
@akshayjshah
Copy link
Contributor

akshayjshah commented Mar 28, 2017

Thanks for the bug report! This should be fixed in #394. Once that lands, I'll cut a new release.

prashantv added a commit that referenced this issue Mar 29, 2017
Config assumes that Open will always return a close function
which causes a panic when Open returns an error, since it doesn't
return a close function.

We can instead clean up the assumption that we return partial values
on error, and instead use a more common pattern:
 - On success, `err == nil` and all other return values are valid
 - On error, `err != nil` and all other return values are zero values

Fixes #390.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants