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.
This commit is contained in:
diamondburned 2021-05-01 17:49:25 -07:00
parent 4e11444f6c
commit 410ac73469
6 changed files with 26 additions and 26 deletions

View File

@ -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.

View File

@ -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

View File

@ -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()
}
}()
}

Binary file not shown.

View File

@ -117,13 +117,13 @@ func (m IOMethod) ReturnError() bool {
return m.ErrorType != ""
}
// ContainerMethod is a method that uses a Container. These methods can do IO
// and always return a stop callback and an error.
// ContainerMethod is a method that uses a Container. These methods can do IO,
// and they must always take in a context and return an error. The context is
// used for both stopping an ongoing IO operation and disconnecting background
// handlers for the container.
type ContainerMethod struct {
method
// HasContext is true if the method accepts a context as its first argument.
HasContext bool
// ContainerType is the name of the container interface. The name will
// almost always have "Container" as its suffix.
ContainerType string

View File

@ -603,7 +603,6 @@ var Main = Packages{
`},
Name: "Name",
},
HasContext: true,
ContainerType: "LabelContainer",
},
},
@ -1069,7 +1068,6 @@ var Main = Packages{
`},
Name: "JoinServer",
},
HasContext: true,
ContainerType: "MessagesContainer",
},
AsserterMethod{ChildType: "Sender"},
@ -1274,7 +1272,6 @@ var Main = Packages{
`},
Name: "ListMembers",
},
HasContext: true,
ContainerType: "MemberListContainer",
},
},
@ -1298,7 +1295,6 @@ var Main = Packages{
`},
Name: "ReadIndicate",
},
HasContext: true,
ContainerType: "ReadContainer",
},
},
@ -1345,7 +1341,6 @@ var Main = Packages{
`},
Name: "UnreadIndicate",
},
HasContext: true,
ContainerType: "UnreadContainer",
},
},
@ -1401,7 +1396,6 @@ var Main = Packages{
`},
Name: "TypingSubscribe",
},
HasContext: true,
ContainerType: "TypingContainer",
},
},