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

[Proposal] Add defer for field resolvers #2860

Closed
h-khodadadeh opened this issue Dec 13, 2023 · 1 comment · Fixed by #2861
Closed

[Proposal] Add defer for field resolvers #2860

h-khodadadeh opened this issue Dec 13, 2023 · 1 comment · Fixed by #2861

Comments

@h-khodadadeh
Copy link

h-khodadadeh commented Dec 13, 2023

What happened?

Running field resolver on tests with mock using github.com/stretchr/testify results in dead-lock, if the called function is not defined.

What did you expect?

My test should run smoothly, no dead-lock.

Minimal graphql.schema and models to reproduce

Create a type like below and define
type Timeout {
Name: String!
}

define a resolver on the field. If you have a resolver, a goroutine will be created to run the resolver. If inside this goroutine runtime.Goexit() is called , then we'll have a dead-lock.

versions

  • go run github.com/99designs/gqlgen version? v0.17.41
  • go version? go version go1.21.0 linux/amd64

Long Version

I have a web server which has many types. Many of these types have resolver on their fields. If you takes a look at the code, you will see that

func (m *FieldSet) Dispatch(ctx context.Context) {
if len(m.delayed) == 1 {
// only one concurrent task, no need to spawn a goroutine or deal create waitgroups
d := m.delayed[0]
m.Values[d.i] = d.f(ctx)
} else if len(m.delayed) > 1 {
// more than one concurrent task, use the main goroutine to do one, only spawn goroutines for the others
var wg sync.WaitGroup
for _, d := range m.delayed[1:] {
wg.Add(1)
go func(d delayedResult) {
m.Values[d.i] = d.f(ctx)
wg.Done()
}(d)
}
m.Values[m.delayed[0].i] = m.delayed[0].f(ctx)
wg.Wait()
}
}

a wait group is defined and it will run resolver functions in a separate goroutine. Once all the resolver functions are finished, wait group allows the code for move forward.

But Sometime Inside the resolver,I call a service which during a test a mock will be used. When the mock doesn't know what to return, it calls FailNow function on *testing.T which in turn calls runtime.Goexit() and it will kill the goroutine. This means that wg.Done() is not called and the application gets stuck at wg.Wait(). To fix this issue, I need to change:

go func(d delayedResult) {
m.Values[d.i] = d.f(ctx)
wg.Done()
}(d)

to

                       go func(d delayedResult) {
                               defer wg.Done()
				m.Values[d.i] = d.f(ctx)
			}(d)

to make sure even if runtime.Goexit() is called, the test will exit instead of running forever.

@h-khodadadeh h-khodadadeh changed the title Add defer for field resolvers [Proposal] Add defer for field resolvers Dec 13, 2023
@UnAfraid
Copy link
Contributor

Nice catch, i've open up a PR to fix it #2861

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

Successfully merging a pull request may close this issue.

2 participants