Panic inside mock MethodCalled method prevent release the mutex

[peczenyj]

Description

In case of a panic inside MethodCalled, the mock mutex is not released. All subsequent calls to mock methods will block and the test will timeout eventually.

This is a problem if we use testing Cleanup to call AssertExpectations -- like what mockery does on each mock constructor.

Step To Reproduce

1. create a mock instance and use testing Cleanup to verify the state
2. perform some action that will eventually panic
3. wait until the test timeout

Expected behavior

I expect having a clear panic ASAP. Or a better expectation/ failure instead a raw panic.

Actual behavior

It panics but the Cleanup blocks the code until timeout, hiding the root cause

Code Example

package foo_test

import (
	"context"
	"testing"

	"github.com/stretchr/testify/mock"
)

type Foo struct {
	mock.Mock
}

func (o *Foo) Do(ctx context.Context) error {
	args := o.Called(ctx)
	return args.Error(0)
}

func NewFoo(t *testing.T) *Foo {
	instance := new(Foo)

	instance.Mock.Test(t)

	t.Cleanup(func() { instance.AssertExpectations(t) })

	return instance
}

func TestFoo(t *testing.T) {
	instance := NewFoo(t)

	var ctx context.Context // ctx is a nil reference, it will panic inside

	instance.On("Do", mock.IsType(ctx)).Return(nil)

	_ = instance.Do(t.Context())
}

then executing

$ go test -timeout=10s -v ./...
=== RUN   TestFoo
panic: test timed out after 10s
	running tests:
		TestFoo (10s)

goroutine 8 [running]:
testing.(*M).startAlarm.func1()
	/usr/local/go1.24.2/src/testing/testing.go:2484 +0x394
created by time.goFunc
	/usr/local/go1.24.2/src/time/sleep.go:215 +0x2d

goroutine 1 [chan receive]:
testing.(*T).Run(0xc0000d0700, {0x64edbe?, 0xc0000ddb30?}, 0x664858)
	/usr/local/go1.24.2/src/testing/testing.go:1859 +0x431
testing.runTests.func1(0xc0000d0700)
	/usr/local/go1.24.2/src/testing/testing.go:2279 +0x37
testing.tRunner(0xc0000d0700, 0xc0000ddc70)
	/usr/local/go1.24.2/src/testing/testing.go:1792 +0xf4
testing.runTests(0xc000010078, {0x839a90, 0x1, 0x1}, {0x8484e0?, 0x7?, 0x847360?})
	/usr/local/go1.24.2/src/testing/testing.go:2277 +0x4b4
testing.(*M).Run(0xc00009edc0)
	/usr/local/go1.24.2/src/testing/testing.go:2142 +0x64a
main.main()
	_testmain.go:45 +0x9b

goroutine 7 [sync.Mutex.Lock]:
internal/sync.runtime_SemacquireMutex(0xc0000a3590?, 0x59?, 0x610e80?)
	/usr/local/go1.24.2/src/runtime/sema.go:95 +0x25
internal/sync.(*Mutex).lockSlow(0xc000014548)
	/usr/local/go1.24.2/src/internal/sync/mutex.go:149 +0x15d
internal/sync.(*Mutex).Lock(...)
	/usr/local/go1.24.2/src/internal/sync/mutex.go:70
sync.(*Mutex).Lock(...)
	/usr/local/go1.24.2/src/sync/mutex.go:46
github.com/stretchr/testify/mock.(*Mock).AssertExpectations(0xc000014500, {0x6a7060, 0xc0000d08c0})
	/home/tiago/work/go/src/path/to/vendor/github.com/stretchr/testify/mock/mock.go:620 +0x114
path/to/foo_test.NewFoo.func1()
	/home/tiago/work/go/src/path/to/foo/foo_test.go:24 +0x25
testing.(*common).Cleanup.func1()
	/usr/local/go1.24.2/src/testing/testing.go:1211 +0x106
testing.(*common).runCleanup(0xc0000d08c0, 0xc0000d0a80?)
	/usr/local/go1.24.2/src/testing/testing.go:1445 +0xea
testing.tRunner.func2()
	/usr/local/go1.24.2/src/testing/testing.go:1786 +0x25
panic({0x60e360?, 0x839120?})
	/usr/local/go1.24.2/src/runtime/panic.go:792 +0x132
github.com/stretchr/testify/mock.Arguments.Diff({0xc00002e590, 0x1, 0x414891?}, {0xc00002e5d0, 0x1, 0xc0000d0a80?})
	/home/tiago/work/go/src/path/to/vendor/github.com/stretchr/testify/mock/mock.go:1005 +0x6e9
github.com/stretchr/testify/mock.(*Mock).findExpectedCall(0x100?, {0x6df40b, 0x2}, {0xc00002e5d0, 0x1, 0x1})
	/home/tiago/work/go/src/path/to/vendor/github.com/stretchr/testify/mock/mock.go:383 +0xf0
github.com/stretchr/testify/mock.(*Mock).MethodCalled(0xc000014500, {0x6df40b, 0x2}, {0xc00002e5d0, 0x1, 0x1})
	/home/tiago/work/go/src/path/to/vendor/github.com/stretchr/testify/mock/mock.go:491 +0xb3
github.com/stretchr/testify/mock.(*Mock).Called(0xc000014500, {0xc00002e5d0, 0x1, 0x1})
	/home/tiago/work/go/src/path/to/vendor/github.com/stretchr/testify/mock/mock.go:481 +0x125
path/to/foo_test.(*Foo).Do(0xc000014500, {0x6a7350, 0xc0000144b0})
	/home/tiago/work/go/src/path/to/foo/foo_test.go:15 +0x7e
path/to/foo_test.TestFoo(0xc0000d08c0)
	/home/tiago/work/go/src/path/to/foo/foo_test.go:36 +0x13a
testing.tRunner(0xc0000d08c0, 0x664858)
	/usr/local/go1.24.2/src/testing/testing.go:1792 +0xf4
created by testing.(*T).Run in goroutine 1
	/usr/local/go1.24.2/src/testing/testing.go:1851 +0x413
FAIL	path/to/foo	10.009s
FAIL

if I comment the t.Cleanup call I have this

$ go test -timeout=10s -v ./...
=== RUN   TestFoo
--- FAIL: TestFoo (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xc8 pc=0x5a89a9]

goroutine 7 [running]:
testing.tRunner.func1.2({0x60d200, 0x837120})
	/usr/local/go1.24.2/src/testing/testing.go:1734 +0x21c
testing.tRunner.func1()
	/usr/local/go1.24.2/src/testing/testing.go:1737 +0x35e
panic({0x60d200?, 0x837120?})
	/usr/local/go1.24.2/src/runtime/panic.go:792 +0x132
github.com/stretchr/testify/mock.Arguments.Diff({0xc00002e590, 0x1, 0x414891?}, {0xc00002e5d0, 0x1, 0xc0000d0a80?})
	/home/tiago/work/go/src/path/to/vendor/github.com/stretchr/testify/mock/mock.go:1005 +0x6e9

I find a possible fix, on the original code:

   484	// MethodCalled tells the mock object that the given method has been called, and gets
   485	// an array of arguments to return. Panics if the call is unexpected (i.e. not preceded
   486	// by appropriate .On .Return() calls)
   487	// If Call.WaitFor is set, blocks until the channel is closed or receives a message.
   488	func (m *Mock) MethodCalled(methodName string, arguments ...interface{}) Arguments {
   489		m.mutex.Lock()
   490		// TODO: could combine expected and closes in single loop
   491		found, call := m.findExpectedCall(methodName, arguments...)

we could do this generic fix to just force release the lock

   484	// MethodCalled tells the mock object that the given method has been called, and gets
   485	// an array of arguments to return. Panics if the call is unexpected (i.e. not preceded
   486	// by appropriate .On .Return() calls)
   487	// If Call.WaitFor is set, blocks until the channel is closed or receives a message.
   488	func (m *Mock) MethodCalled(methodName string, arguments ...interface{}) Arguments {
   489		m.mutex.Lock()
                defer func(){
                    if r := recover(); r != nil {
                        _ = m.mutex.TryUnlock() // REQUIRES go 1.18
                       panic(r)                 // throw the panic again
                    }
                }()
   490		// TODO: could combine expected and closes in single loop
   491		found, call := m.findExpectedCall(methodName, arguments...)