From 461e407fbed90d546c828b2527356312fc95051e Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 9 May 2022 13:19:44 +0530 Subject: [PATCH] MM-44044: Fix racy TestGraphQLChannelMembers (#20155) Upgrading the user resolver to dataloader meant that multiple goroutines would access the user struct. Hence we move the user struct sanitization inside the dataloader itself so prevent access outside the dataloader. https://mattermost.atlassian.net/browse/MM-44044 ```release-note NONE ``` --- api4/resolver_user.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/api4/resolver_user.go b/api4/resolver_user.go index 115d74e06f..a1058054cd 100644 --- a/api4/resolver_user.go +++ b/api4/resolver_user.go @@ -56,11 +56,6 @@ func getGraphQLUser(ctx context.Context, id string) (*user, error) { } } - if c.AppContext.Session().UserId == usr.Id { - usr.Sanitize(map[string]bool{}) - } else { - c.App.SanitizeProfile(usr, c.IsSystemAdmin()) - } c.App.UpdateLastActivityAtIfNeeded(*c.AppContext.Session()) return &user{*usr}, nil @@ -199,6 +194,18 @@ func getGraphQLUsers(c *web.Context, userIDs []string) ([]*model.User, error) { return nil, appErr } + // Same as earlier, we want to pre-compute this only once + // because otherwise the resolvers run in multiple goroutines + // and *User.Sanitize causes a race, and we want to avoid + // deep-copying every user in all goroutines. + for _, user := range users { + if c.AppContext.Session().UserId == user.Id { + user.Sanitize(map[string]bool{}) + } else { + c.App.SanitizeProfile(user, c.IsSystemAdmin()) + } + } + // The users need to be in the exact same order as the input slice. tmp := make(map[string]*model.User) for _, u := range users {