app: avoid plugin env check for RunMultiHook and use pluginslock while accessing plugin env (#21803)

* app/channels: use pluginslock while accessing plugins environment

* when using RunMultiHook we don't need to do a nil check on plugin env

* trigger ci

app/channel.go

@@ -343,7 +343,6 @@ func (a *App) CreateChannel(c request.CTX, channel *model.Channel, addMember boo
 		a.InvalidateCacheForUser(channel.CreatorId)
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	a.Srv().Go(func() {
 		pluginContext := pluginContext(c)
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -351,7 +350,6 @@ func (a *App) CreateChannel(c request.CTX, channel *model.Channel, addMember boo
 			return true
 		}, plugin.ChannelHasBeenCreatedID)
 	})
-	}
 
 	return sc, nil
 }
@@ -429,7 +427,6 @@ func (a *App) handleCreationEvent(c request.CTX, userID, otherUserID string, cha
 	a.InvalidateCacheForUser(userID)
 	a.InvalidateCacheForUser(otherUserID)
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	a.Srv().Go(func() {
 		pluginContext := pluginContext(c)
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -437,7 +434,6 @@ func (a *App) handleCreationEvent(c request.CTX, userID, otherUserID string, cha
 			return true
 		}, plugin.ChannelHasBeenCreatedID)
 	})
-	}
 
 	message := model.NewWebSocketEvent(model.WebsocketEventDirectAdded, "", channel.Id, "", nil, "")
 	message.Add("creator_id", userID)
@@ -1599,7 +1595,6 @@ func (a *App) AddChannelMember(c request.CTX, userID string, channel *model.Chan
 		return nil, err
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	a.Srv().Go(func() {
 		pluginContext := pluginContext(c)
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -1607,7 +1602,6 @@ func (a *App) AddChannelMember(c request.CTX, userID string, channel *model.Chan
 			return true
 		}, plugin.UserHasJoinedChannelID)
 	})
-	}
 
 	if opts.UserRequestorID == "" || userID == opts.UserRequestorID {
 		if err := a.postJoinChannelMessage(c, user, channel); err != nil {
@@ -2177,7 +2171,6 @@ func (a *App) JoinChannel(c request.CTX, channel *model.Channel, userID string)
 		return err
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	a.Srv().Go(func() {
 		pluginContext := pluginContext(c)
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -2185,7 +2178,6 @@ func (a *App) JoinChannel(c request.CTX, channel *model.Channel, userID string)
 			return true
 		}, plugin.UserHasJoinedChannelID)
 	})
-	}
 
 	if err := a.postJoinChannelMessage(c, user, channel); err != nil {
 		return err
@@ -2484,7 +2476,6 @@ func (a *App) removeUserFromChannel(c request.CTX, userIDToRemove string, remove
 	a.InvalidateCacheForUser(userIDToRemove)
 	a.invalidateCacheForChannelMembers(channel.Id)
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	var actorUser *model.User
 	if removerUserId != "" {
 		actorUser, _ = a.GetUser(removerUserId)
@@ -2497,7 +2488,6 @@ func (a *App) removeUserFromChannel(c request.CTX, userIDToRemove string, remove
 			return true
 		}, plugin.UserHasLeftChannelID)
 	})
-	}
 
 	message := model.NewWebSocketEvent(model.WebsocketEventUserRemoved, "", channel.Id, "", nil, "")
 	message.Add("user_id", userIDToRemove)

app/channels.go

@@ -326,7 +326,7 @@ func (s *hooksService) RegisterHooks(productID string, hooks any) error {
 }
 
 func (ch *Channels) RunMultiHook(hookRunnerFunc func(hooks plugin.Hooks) bool, hookId int) {
-	if env := ch.pluginsEnvironment; env != nil {
+	if env := ch.GetPluginsEnvironment(); env != nil {
 		env.RunMultiPluginHook(hookRunnerFunc, hookId)
 	}
 
@@ -336,7 +336,7 @@ func (ch *Channels) RunMultiHook(hookRunnerFunc func(hooks plugin.Hooks) bool, h
 
 func (ch *Channels) HooksForPluginOrProduct(id string) (plugin.Hooks, error) {
 	var hooks plugin.Hooks
-	if env := ch.pluginsEnvironment; env != nil {
+	if env := ch.GetPluginsEnvironment(); env != nil {
 		// we intentionally ignore the error here, because the id can be a product id
 		// we are going to check if we have the hooks or not
 		hooks, _ = env.HooksForPlugin(id)

app/file.go

@@ -895,7 +895,6 @@ func (a *App) DoUploadFileExpectModification(c request.CTX, now time.Time, rawTe
 		info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb." + getFileExtFromMimeType(info.MimeType)
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	var rejectionError *model.AppError
 	pluginContext := pluginContext(c)
 	a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -918,7 +917,6 @@ func (a *App) DoUploadFileExpectModification(c request.CTX, now time.Time, rawTe
 	if rejectionError != nil {
 		return nil, data, rejectionError
 	}
-	}
 
 	if _, err := a.WriteFile(bytes.NewReader(data), info.Path); err != nil {
 		return nil, data, err

app/login.go

@@ -157,7 +157,6 @@ func (a *App) GetUserForLogin(id, loginId string) (*model.User, *model.AppError)
 }
 
 func (a *App) DoLogin(c *request.Context, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile, isOAuthUser, isSaml bool) *model.AppError {
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	var rejectionReason string
 	pluginContext := pluginContext(c)
 	a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -168,7 +167,6 @@ func (a *App) DoLogin(c *request.Context, w http.ResponseWriter, r *http.Request
 	if rejectionReason != "" {
 		return model.NewAppError("DoLogin", "Login rejected by plugin: "+rejectionReason, nil, "", http.StatusBadRequest)
 	}
-	}
 
 	session := &model.Session{UserId: user.Id, Roles: user.GetRawRoles(), DeviceId: deviceID, IsOAuth: false, Props: map[string]string{
 		model.UserAuthServiceIsMobile: strconv.FormatBool(isMobile),
@@ -226,15 +224,12 @@ func (a *App) DoLogin(c *request.Context, w http.ResponseWriter, r *http.Request
 		})
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	a.Srv().Go(func() {
-			pluginContext := pluginContext(c)
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
 			hooks.UserHasLoggedIn(pluginContext, user)
 			return true
 		}, plugin.UserHasLoggedInID)
 	})
-	}
 
 	return nil
 }

app/plugin.go

@@ -286,14 +286,13 @@ func (ch *Channels) initPlugins(c *request.Context, pluginDir, webappPluginDir s
 			ch.installFeatureFlagPlugins()
 			ch.syncPluginsActiveState()
 		}
-		if pluginsEnvironment := ch.GetPluginsEnvironment(); pluginsEnvironment != nil {
+
 		ch.RunMultiHook(func(hooks plugin.Hooks) bool {
 			if err := hooks.OnConfigurationChange(); err != nil {
 				ch.srv.Log().Error("Plugin OnConfigurationChange hook failed", mlog.Err(err))
 			}
 			return true
 		}, plugin.OnConfigurationChangeID)
-		}
 	})
 	ch.pluginsLock.Unlock()
 

app/post.go

@@ -263,7 +263,6 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel
 		post.Metadata.Priority = nil
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	var metadata *model.PostMetadata
 	if post.Metadata != nil {
 		metadata = post.Metadata.Copy()
@@ -295,7 +294,6 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel
 	if rejectionError != nil {
 		return nil, rejectionError
 	}
-	}
 
 	// Pre-fill the CreateAt field for link previews to get the correct timestamp.
 	if post.CreateAt == 0 {
@@ -328,16 +326,13 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel
 
 	// We make a copy of the post for the plugin hook to avoid a race condition,
 	// and to remove the non-GOB-encodable Metadata from it.
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	pluginPost := rpost.ForPlugin()
 	a.Srv().Go(func() {
-			pluginContext := pluginContext(c)
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
 			hooks.MessageHasBeenPosted(pluginContext, pluginPost)
 			return true
 		}, plugin.MessageHasBeenPostedID)
 	})
-	}
 
 	if a.Metrics() != nil {
 		a.Metrics().IncrementPostCreate()
@@ -658,7 +653,6 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool)
 		oldPost.RemoteId = model.NewString(*post.RemoteId)
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	var rejectionReason string
 	pluginContext := pluginContext(c)
 	a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -671,7 +665,6 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool)
 	// Restore the post metadata that was stripped by the plugin. Set it to
 	// the last known good.
 	newPost.Metadata = oldPost.Metadata
-	}
 
 	rpost, nErr := a.Srv().Store().Post().Update(newPost, oldPost)
 	if nErr != nil {
@@ -684,17 +677,14 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool)
 		}
 	}
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	pluginOldPost := oldPost.ForPlugin()
 	pluginNewPost := newPost.ForPlugin()
 	a.Srv().Go(func() {
-			pluginContext := pluginContext(c)
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
 			hooks.MessageHasBeenUpdated(pluginContext, pluginNewPost, pluginOldPost)
 			return true
 		}, plugin.MessageHasBeenUpdatedID)
 	})
-	}
 
 	rpost = a.PreparePostForClientWithEmbedsAndImages(c, rpost, false, true, true)
 

app/reaction.go

@@ -43,15 +43,13 @@ func (a *App) SaveReactionForPost(c *request.Context, reaction *model.Reaction)
 	// The post is always modified since the UpdateAt always changes
 	a.invalidateCacheForChannelPosts(post.ChannelId)
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
-		a.Srv().Go(func() {
 	pluginContext := pluginContext(c)
+	a.Srv().Go(func() {
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
 			hooks.ReactionHasBeenAdded(pluginContext, reaction)
 			return true
 		}, plugin.ReactionHasBeenAddedID)
 	})
-	}
 
 	a.Srv().Go(func() {
 		a.sendReactionEvent(model.WebsocketEventReactionAdded, reaction, post)
@@ -142,15 +140,13 @@ func (a *App) DeleteReactionForPost(c *request.Context, reaction *model.Reaction
 	// The post is always modified since the UpdateAt always changes
 	a.invalidateCacheForChannelPosts(post.ChannelId)
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
-		a.Srv().Go(func() {
 	pluginContext := pluginContext(c)
+	a.Srv().Go(func() {
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
 			hooks.ReactionHasBeenRemoved(pluginContext, reaction)
 			return true
 		}, plugin.ReactionHasBeenRemovedID)
 	})
-	}
 
 	a.Srv().Go(func() {
 		a.sendReactionEvent(model.WebsocketEventReactionRemoved, reaction, post)

app/team.go

@@ -846,7 +846,6 @@ func (a *App) JoinUserToTeam(c request.CTX, team *model.Team, user *model.User,
 	a.InvalidateCacheForUser(user.Id)
 	a.invalidateCacheForUserTeams(user.Id)
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	var actor *model.User
 	if userRequestorId != "" {
 		actor, _ = a.GetUser(userRequestorId)
@@ -859,7 +858,6 @@ func (a *App) JoinUserToTeam(c request.CTX, team *model.Team, user *model.User,
 			return true
 		}, plugin.UserHasJoinedTeamID)
 	})
-	}
 
 	message := model.NewWebSocketEvent(model.WebsocketEventAddedToTeam, "", "", user.Id, nil, "")
 	message.Add("team_id", team.Id)
@@ -1220,7 +1218,6 @@ func (a *App) RemoveUserFromTeam(c request.CTX, teamID string, userID string, re
 }
 
 func (a *App) postProcessTeamMemberLeave(c request.CTX, teamMember *model.TeamMember, requestorId string) *model.AppError {
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
 	var actor *model.User
 	if requestorId != "" {
 		actor, _ = a.GetUser(requestorId)
@@ -1233,7 +1230,6 @@ func (a *App) postProcessTeamMemberLeave(c request.CTX, teamMember *model.TeamMe
 			return true
 		}, plugin.UserHasLeftTeamID)
 	})
-	}
 
 	user, nErr := a.Srv().Store().User().Get(context.Background(), teamMember.UserId)
 	if nErr != nil {

app/upload.go

@@ -49,11 +49,6 @@ func (a *App) genFileInfoFromReader(name string, file io.ReadSeeker, size int64)
 }
 
 func (a *App) runPluginsHook(c *request.Context, info *model.FileInfo, file io.Reader) *model.AppError {
-	pluginsEnvironment := a.GetPluginsEnvironment()
-	if pluginsEnvironment == nil {
-		return nil
-	}
-
 	filePath := info.Path
 	// using a pipe to avoid loading the whole file content in memory.
 	r, w := io.Pipe()

app/user.go

@@ -308,15 +308,13 @@ func (a *App) createUserOrGuest(c request.CTX, user *model.User, guest bool) (*m
 	message.Add("user_id", ruser.Id)
 	a.Publish(message)
 
-	if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
-		a.Srv().Go(func() {
 	pluginContext := pluginContext(c)
+	a.Srv().Go(func() {
 		a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
 			hooks.UserHasBeenCreated(pluginContext, ruser)
 			return true
 		}, plugin.UserHasBeenCreatedID)
 	})
-	}
 
 	_, cwsErr := a.SendSubscriptionHistoryEvent(ruser.Id)
 	if cwsErr != nil {