From 410ac734699a9db5d52b34b73ff4cde79729e677 Mon Sep 17 00:00:00 2001 From: diamondburned Date: Sat, 1 May 2021 17:49:25 -0700 Subject: [PATCH] Replace stop callbacks with contexts This commit removes all stop callbacks in ContainerMethods. The intention is to have backends disconnect callbacks when the context is cancelled, rather than when the stop function is called. This helps get rid of countless race condition flaws caused by the duration between the context being cancelled on one thread and the stop callback being set in another, causing the handlers to not disconnect. --- cchat.go | 14 +++++++------- .../cchat-generator/generate_interface.go | 11 ++--------- generator.go | 13 +++++++++++++ repository/gob/repository.gob | Bin 38151 -> 38124 bytes repository/interface.go | 8 ++++---- repository/main.go | 6 ------ 6 files changed, 26 insertions(+), 26 deletions(-) diff --git a/cchat.go b/cchat.go index 5a28dd5..f0c9ccc 100644 --- a/cchat.go +++ b/cchat.go @@ -392,7 +392,7 @@ type Lister interface { // Servers should call SetServers() on the given ServersContainer to render all // servers. This function can do IO, and the frontend should run this in a // goroutine. - Servers(ServersContainer) (stop func(), err error) + Servers(context.Context, ServersContainer) error // Columnate is optionally used by servers to tell the frontend whether or not // its children should be put onto a new column instead of underneath it within // the same tree. If the method returns false, then the frontend can treat its @@ -477,7 +477,7 @@ type MemberLister interface { // frontends must not rely solely on this, as the general context rules applies. // // Further behavioral documentations may be in Messenger's JoinServer method. - ListMembers(context.Context, MemberListContainer) (stop func(), err error) + ListMembers(context.Context, MemberListContainer) error } // MemberSection represents a member list section. The section name's content @@ -557,7 +557,7 @@ type Messenger interface { // backend can safely assume that there will only ever be one active JoinServer. // If the frontend wishes to do this, it must keep its own shared message // buffer. - JoinServer(context.Context, MessagesContainer) (stop func(), err error) + JoinServer(context.Context, MessagesContainer) error // Asserters. @@ -581,7 +581,7 @@ type Namer interface { // Name sets the given container to contain the name of the parent context. The // method has no stop method; stopping is implied to be dependent on the parent // context. As such, it's only used for updating. - Name(context.Context, LabelContainer) (stop func(), err error) + Name(context.Context, LabelContainer) error } // Nicknamer adds the current user's nickname. @@ -632,7 +632,7 @@ type ReadIndicator interface { // must keep track of which read states to send over to not overwhelm the // frontend, and the frontend must either keep track of them, or it should not // display it at all. - ReadIndicate(context.Context, ReadContainer) (stop func(), err error) + ReadIndicate(context.Context, ReadContainer) error } // Replier indicates that the message being sent is a reply to something. @@ -857,7 +857,7 @@ type TypingIndicator interface { // This method does not take in a context, as it's supposed to only use event // handlers and not do any IO calls. Nonetheless, the client must treat it like // it does and call it asynchronously. - TypingSubscribe(context.Context, TypingContainer) (stop func(), err error) + TypingSubscribe(context.Context, TypingContainer) error // TypingTimeout returns the interval between typing events sent by the client // as well as the timeout before the client should remove the typer. Typically, // a constant should be returned. @@ -899,7 +899,7 @@ type UnreadIndicator interface { // // This function must provide a way to remove callbacks, as clients must call // this when the old server is destroyed, such as when Servers is called. - UnreadIndicate(context.Context, UnreadContainer) (stop func(), err error) + UnreadIndicate(context.Context, UnreadContainer) error // MarkRead marks a message in the server messenger as read. Backends that // implement the UnreadIndicator interface must give control of marking messages // as read to the frontend if possible. diff --git a/cmd/internal/cchat-generator/generate_interface.go b/cmd/internal/cchat-generator/generate_interface.go index e0ebf00..553993a 100644 --- a/cmd/internal/cchat-generator/generate_interface.go +++ b/cmd/internal/cchat-generator/generate_interface.go @@ -156,20 +156,13 @@ func generateFuncParams(params []repository.NamedType, errorType string) []jen.C } func generateContainerFuncReturns(method repository.ContainerMethod) []jen.Code { - var stmt jen.Statement - - stmt.Add(jen.Id("stop").Func().Params()) - stmt.Add(jen.Err().Error()) - - return stmt + return []jen.Code{jen.Error()} } func generateContainerFuncParams(method repository.ContainerMethod) []jen.Code { var stmt jen.Statement - if method.HasContext { - stmt.Qual("context", "Context") - } + stmt.Qual("context", "Context") stmt.Add(genutils.GenerateType(method)) return stmt diff --git a/generator.go b/generator.go index 69518d9..28dedb7 100644 --- a/generator.go +++ b/generator.go @@ -1,5 +1,7 @@ package cchat +import "context" + //go:generate go run ./cmd/internal/cchat-generator ./ //go:generate go run ./cmd/internal/cchat-empty-gen ./utils/empty/ @@ -16,3 +18,14 @@ func WrapAuthenticateError(err error) AuthenticateError { } return authenticateError{err} } + +// CtxCallbacks binds a set of given callbacks to the given context. This is +// useful for disconnecting handlers when the context expires. +func CtxCallbacks(ctx context.Context, fns ...func()) { + go func() { + <-ctx.Done() + for _, fn := range fns { + fn() + } + }() +} diff --git a/repository/gob/repository.gob b/repository/gob/repository.gob index b608ccb586a0632f5c4295a280e3f629de0c61a6..0eb4c60f207db91d837fe4be045899bff289f539 100644 GIT binary patch delta 109 zcmV-z0FwWQssik(0s)0Ras|Z*Fv9X>Mh5RC#b^0So{D0R9f4|F{ve z-aKg$1-1XY9s!fUO%=0YPCN|(0h69v$N?0SplB4ceO%ED0T#1SX;A?I5VNIfwgCYW PvtDeB0s#`U?QR+-P{JmC delta 136 zcmaE}lBs