stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
22.5k stars 1.56k forks source link

mock: Deadlock if mocking String() #643

Open lily-mara opened 5 years ago

lily-mara commented 5 years ago

This is a fairly contrived example, I admit. But this was a real head-scratcher in the wild.

The following code will never complete.

package main

import (
    "fmt"

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

type K struct {
    mock.Mock
}

func (f *K) String() string {
    return f.Called().Get(0).(string)
}

func (f *K) Foo(b B) string {
    return f.Called(b).Get(0).(string)
}

type B struct {
    Kay *K
}

func main() {
    k := &K{}
    b := B{
        Kay: k,
    }

    k.On("String").Return("string-return")
    k.On("Foo", b).Return("foo-return")

    fmt.Printf(k.Foo(b))
}

This fails because Mock.Called calls Mock.MethodCalled, which locks the Mutex, and eventually calls Arguments.Diff. When Arguments.Diff calls fmt.Sprintf, fmt.Sprintf sees the Kay field of B. Since K implements fmt.Stringer, fmt.Sprintf calls K.String. Since this is a mocked function, K.String jumps into Mock.Called, which calls Mock.MethodCalled which waits for the still-locked mutex to unlock, which never happens, because the lock is further up the call chain.

I don't know if this should be considered a bug or not, but it certainly was unexpected, and took a lot of digging to figure out.

lily-mara commented 5 years ago

Here's a stack trace from a debug session just before the mutex lock deadlock

github.com/stretchr/testify/mock.(*Mock).MethodCalled (github.com/stretchr/testify/mock/mock.go:318)
github.com/stretchr/testify/mock.(*Mock).Called (github.com/stretchr/testify/mock/mock.go:310)
main.(*K).String (main.go)
fmt.(*pp).handleMethods (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:603)
fmt.(*pp).printValue (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:700)
fmt.(*pp).printValue (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:783)
fmt.(*pp).printValue (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:853)
fmt.(*pp).printArg (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:689)
fmt.(*pp).doPrintf (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:1003)
fmt.Sprintf (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:203)
github.com/stretchr/testify/mock.Arguments.Diff (github.com/stretchr/testify/mock/mock.go:675)
github.com/stretchr/testify/mock.(*Mock).findExpectedCall (github.com/stretchr/testify/mock/mock.go:242)
github.com/stretchr/testify/mock.(*Mock).MethodCalled (github.com/stretchr/testify/mock/mock.go:319)
github.com/stretchr/testify/mock.(*Mock).Called (github.com/stretchr/testify/mock/mock.go:310)
main.(*K).Foo (main.go)
main.main (main.go)