Update after comments

config_test.go

@@ -68,17 +68,13 @@ func TestConfig(t *testing.T) {
 			logger, err := tt.cfg.Build(Hooks(hook))
 			require.NoError(t, err, "Unexpected error constructing logger.")
 
-			withStacktraceIgnorePrefixes([]string{}, func() {
+			withNoStacktraceIgnorePrefixes(func() {
 				logger.Debug("debug")
 				logger.Info("info")
 				logger.Warn("warn")
 
 				byteContents, err := ioutil.ReadAll(temp)
-				// not doing require so no problem with lock in withStacktraceIgnorePrefixes
-				assert.NoError(t, err, "Couldn't read log contents from temp file.")
-				if err != nil {
-					return
-				}
+				require.NoError(t, err, "Couldn't read log contents from temp file.")
 				logs := string(byteContents)
 				assert.Regexp(t, tt.expectRe, logs, "Unexpected log output.")
 

field_test.go

@@ -164,7 +164,7 @@ func TestFieldConstructors(t *testing.T) {
 }
 
 func TestStackField(t *testing.T) {
-	withStacktraceIgnorePrefixes([]string{}, func() {
+	withNoStacktraceIgnorePrefixes(func() {
 		f := Stack("stacktrace")
 		assert.Equal(t, "stacktrace", f.Key, "Unexpected field key.")
 		assert.Equal(t, zapcore.StringType, f.Type, "Unexpected field type.")

logger_test.go

@@ -360,7 +360,7 @@ func TestLoggerAddCallerFail(t *testing.T) {
 }
 
 func TestLoggerAddStacktrace(t *testing.T) {
-	withStacktraceIgnorePrefixes([]string{}, func() {
+	withNoStacktraceIgnorePrefixes(func() {
 		assertHasStack := func(t testing.TB, obs observer.LoggedEntry) {
 			assert.Contains(t, obs.Entry.Stack, "zap.TestLoggerAddStacktrace", "Expected to find test function in stacktrace.")
 		}

stacktrace.go

@@ -31,11 +31,12 @@ import (
 var (
 	_stacktraceIgnorePrefixes = []string{
 		"go.uber.org/zap",
-		"runtime.",
+		"runtime.goexit",
+		"runtime.main",
 	}
-	_stacktraceUintptrPool = sync.Pool{
+	_stacktracePool = sync.Pool{
 		New: func() interface{} {
-			return make([]uintptr, 64)
+			return newProgramCounters(64)
 		},
 	}
 )
@@ -45,24 +46,26 @@ var (
 // successively larger slices until it can capture the whole stack.
 func takeStacktrace() string {
 	buffer := bufferpool.Get()
-	programCounters := _stacktraceUintptrPool.Get().([]uintptr)
 	defer bufferpool.Put(buffer)
-	defer _stacktraceUintptrPool.Put(programCounters)
+	programCounters := _stacktracePool.Get().(*programCounters)
+	defer _stacktracePool.Put(programCounters)
 
 	for {
-		n := runtime.Callers(2, programCounters)
-		if n < len(programCounters) {
-			programCounters = programCounters[:n]
+		// skipping 2 skips the call to runtime.Counters and takeStacktrace
+		// so that the program counters start at the caller of takeStacktrace
+		n := runtime.Callers(2, programCounters.pcs)
+		if n < len(programCounters.pcs) {
+			programCounters.pcs = programCounters.pcs[:n]
 			break
 		}
 		// Do not put programCounters back in pool, will put larger buffer in
 		// This is in case our size is always wrong, we optimize to pul
 		// correctly-sized buffers back in the pool
-		programCounters = make([]uintptr, len(programCounters)*2)
+		programCounters = newProgramCounters(len(programCounters.pcs) * 2)
 	}
 
 	i := 0
-	for _, programCounter := range programCounters {
+	for _, programCounter := range programCounters.pcs {
 		f := runtime.FuncForPC(programCounter)
 		name := f.Name()
 		if shouldIgnoreStacktraceName(name) {
@@ -92,3 +95,11 @@ func shouldIgnoreStacktraceName(name string) bool {
 	}
 	return false
 }
+
+type programCounters struct {
+	pcs []uintptr
+}
+
+func newProgramCounters(size int) *programCounters {
+	return &programCounters{make([]uintptr, size)}
+}

stacktrace_test.go

@@ -46,8 +46,7 @@ func TestEmptyStacktrace(t *testing.T) {
 }
 
 func TestAllStacktrace(t *testing.T) {
-	withStacktraceIgnorePrefixes(
-		[]string{},
+	withNoStacktraceIgnorePrefixes(
 		func() {
 			stacktrace := takeStacktrace()
 			assert.True(t, strings.HasPrefix(stacktrace, "go.uber.org/zap.TestAllStacktrace"),
@@ -56,13 +55,17 @@ func TestAllStacktrace(t *testing.T) {
 	)
 }
 
-// What happens with require functions? If they completely break out of the
-// function, will this lock never get unlocked?
+func withNoStacktraceIgnorePrefixes(f func()) {
+	withStacktraceIgnorePrefixes([]string{}, f)
+}
+
 func withStacktraceIgnorePrefixes(prefixes []string, f func()) {
 	_stacktraceTestLock.Lock()
+	defer _stacktraceTestLock.Unlock()
 	existing := _stacktraceIgnorePrefixes
 	_stacktraceIgnorePrefixes = prefixes
+	defer func() {
+		_stacktraceIgnorePrefixes = existing
+	}()
 	f()
-	_stacktraceIgnorePrefixes = existing
-	_stacktraceTestLock.Unlock()
 }