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

@@ -176,7 +176,7 @@ type Actioner interface {
 	// Do executes a message action on the given messageID, which would be taken
 	// from MessageHeader.ID(). This method is allowed to do IO; the frontend should
 	// take care of running it asynchronously.
-	Do(action string, id ID) error // Blocking
+	Do(ctx context.Context, action string, id ID) error // Blocking
 	// MessageActions returns a list of possible actions to a message in pretty
 	// strings that the frontend will use to directly display. This method must not
 	// do IO.
@@ -217,7 +217,7 @@ type AuthenticateError interface {
 type Authenticator interface {
 	// Authenticate will be called with a list of values with indices correspond to
 	// the returned slice of AuthenticateEntry.
-	Authenticate([]string) (Session, AuthenticateError) // Blocking
+	Authenticate(context.Context, []string) (Session, AuthenticateError) // Blocking
 	// AuthenticateForm should return a list of authentication entries for the
 	// frontend to render.
 	AuthenticateForm() []AuthenticateEntry
@@ -295,7 +295,7 @@ type Commander interface {
 	// A helper function for this kind of behavior is available in package split,
 	// under the ArgsIndexed function. This implementation also provides the rough
 	// specifications.
-	Run(words []string) ([]byte, error) // Blocking
+	Run(ctx context.Context, words []string) ([]byte, error) // Blocking
 
 	// Asserters.
 
@@ -318,19 +318,17 @@ type Completer interface {
 }
 
 // Configurator is an interface which the backend can implement for a primitive
-// configuration API. Since these methods do return an error, they are allowed
-// to do IO. The frontend should handle this appropriately, including running
-// them asynchronously.
+// configuration API.
 type Configurator interface {
-	SetConfiguration(map[string]string) error  // Blocking
-	Configuration() (map[string]string, error) // Blocking
+	SetConfiguration(map[string]string) error
+	Configuration() map[string]string
 }
 
 // Editor adds message editing to the messenger. Only EditMessage can do IO.
 type Editor interface {
 	// Edit edits the message with the given ID to the given content, which is the
 	// edited string from RawMessageContent. This method can do IO.
-	Edit(id ID, content string) error // Blocking
+	Edit(ctx context.Context, id ID, content string) error // Blocking
 	// RawContent gets the original message text for editing. This method must not
 	// do IO.
 	RawContent(id ID) (string, error)
@@ -394,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
@@ -421,14 +419,14 @@ type MemberDynamicSection interface {
 	// The client can call this method exactly as many times as it has called
 	// LoadMore. However, false should be returned if the client should stop, and
 	// future calls without LoadMore should still return false.
-	LoadLess() bool // Blocking
+	LoadLess(context.Context) bool // Blocking
 	// LoadMore is a method which the client can call to ask for more members. This
 	// method can do IO.
 	//
 	// Clients may call this method on the last section in the section slice;
 	// however, calling this method on any section is allowed. Clients may not call
 	// this method if the number of members in this section is equal to Total.
-	LoadMore() bool // Blocking
+	LoadMore(context.Context) bool // Blocking
 }
 
 // MemberListContainer is a generic interface for any container that can display
@@ -479,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
@@ -559,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.
 
@@ -583,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.
@@ -634,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(ReadContainer) (stop func(), err error)
+	ReadIndicate(context.Context, ReadContainer) error
 }
 
 // Replier indicates that the message being sent is a reply to something.
@@ -668,7 +666,7 @@ type Sender interface {
 	// CanAttach returns whether or not the client is allowed to upload files.
 	CanAttach() bool
 	// Send is called by the frontend to send a message to this channel.
-	Send(SendableMessage) error // Blocking
+	Send(context.Context, SendableMessage) error // Blocking
 
 	// Asserters.
 
@@ -796,7 +794,7 @@ type Session interface {
 	// When this function fails, the frontend may display the error upfront.
 	// However, it will treat the session as actually disconnected. If needed, the
 	// backend must implement reconnection by itself.
-	Disconnect() error // Blocking, Disposer
+	Disconnect(context.Context) error // Blocking, Disposer
 
 	// Asserters.
 
@@ -810,7 +808,7 @@ type Session interface {
 //
 // To save a session, refer to SessionSaver.
 type SessionRestorer interface {
-	RestoreSession(map[string]string) (Session, error) // Blocking
+	RestoreSession(context.Context, map[string]string) (Session, error) // Blocking
 }
 
 // SessionSaver extends Session and is called by the frontend to save the
@@ -859,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(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.
@@ -868,7 +866,7 @@ type TypingIndicator interface {
 	// function can do IO calls, and the client must take care of calling it in a
 	// goroutine (or an asynchronous queue) as well as throttling it to
 	// TypingTimeout.
-	Typing() error // Blocking
+	Typing(context.Context) error // Blocking
 }
 
 // UnreadContainer is an interface that a single server container (such as a
@@ -901,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(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.

cmd/internal/cchat-generator/generate_interface.go

@@ -66,9 +66,10 @@ func generateInterfaces(ifaces []repository.Interface) jen.Code {
 					stmt.Params(generateFuncParams(method.Returns, method.ErrorType)...)
 				case repository.SetterMethod:
 					stmt.Params(generateFuncParams(method.Parameters, "")...)
+					stmt.Params(generateFuncParamsErr(repository.NamedType{}, method.ErrorType)...)
 				case repository.IOMethod:
-					stmt.Params(generateFuncParams(method.Parameters, "")...)
-					stmt.Params(generateFuncParamErr(method.ReturnValue, method.ErrorType)...)
+					stmt.Params(generateFuncParamsCtx(method.Parameters, "")...)
+					stmt.Params(generateFuncParamsErr(method.ReturnValue, method.ErrorType)...)
 					var comment = "Blocking"
 					if method.Disposer {
 						comment += ", Disposer"
@@ -96,7 +97,7 @@ func generateInterfaces(ifaces []repository.Interface) jen.Code {
 	return stmt
 }
 
-func generateFuncParamErr(param repository.NamedType, errorType string) []jen.Code {
+func generateFuncParamsErr(param repository.NamedType, errorType string) []jen.Code {
 	stmt := make([]jen.Code, 0, 2)
 
 	if !param.IsZero() {
@@ -121,6 +122,18 @@ func generateFuncParam(param repository.NamedType) jen.Code {
 	return jen.Id(param.Name).Add(genutils.GenerateType(param))
 }
 
+func generateFuncParamsCtx(params []repository.NamedType, errorType string) []jen.Code {
+	var name string
+	if len(params) > 0 && params[0].Name != "" {
+		name = "ctx"
+	}
+
+	p := []repository.NamedType{{Name: name, Type: "context.Context"}}
+	p = append(p, params...)
+
+	return generateFuncParams(p, errorType)
+}
+
 func generateFuncParams(params []repository.NamedType, errorType string) []jen.Code {
 	if len(params) == 0 {
 		return nil
@@ -143,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

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

repository/interface.go

@@ -78,10 +78,16 @@ type SetterMethod struct {
 	// Parameters is the list of parameters in the function. These parameters
 	// should be the parameters to set.
 	Parameters []NamedType
+	// ErrorType is non-empty if the function returns an error at the end of
+	// returns. An error may be returned from the backend if the input is
+	// invalid, but it must not do IO. Frontend setters must never error.
+	ErrorType string
 }
 
 // IOMethod is a regular method that can do IO and thus is blocking. These
-// methods usually always return errors.
+// methods usually always return errors. IOMethods must always have means of
+// cancelling them in the API, but implementations don't have to use it; as
+// such, the user should always have a timeout to gracefully wait.
 type IOMethod struct {
 	method
 
@@ -111,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

repository/main.go

@@ -603,7 +603,6 @@ var Main = Packages{
 						`},
 						Name: "Name",
 					},
-					HasContext:    true,
 					ContainerType: "LabelContainer",
 				},
 			},
@@ -817,18 +816,15 @@ var Main = Packages{
 		}, {
 			Comment: Comment{`
 				Configurator is an interface which the backend can implement for a
-				primitive configuration API. Since these methods do return an error,
-				they are allowed to do IO. The frontend should handle this
-				appropriately, including running them asynchronously.
+				primitive configuration API.
 			`},
 			Name: "Configurator",
 			Methods: []Method{
-				IOMethod{
-					method:      method{Name: "Configuration"},
-					ReturnValue: NamedType{Type: "map[string]string"},
-					ErrorType:   "error",
+				GetterMethod{
+					method:  method{Name: "Configuration"},
+					Returns: []NamedType{{Type: "map[string]string"}},
 				},
-				IOMethod{
+				SetterMethod{
 					method:     method{Name: "SetConfiguration"},
 					Parameters: []NamedType{{Type: "map[string]string"}},
 					ErrorType:  "error",
@@ -1072,7 +1068,6 @@ var Main = Packages{
 						`},
 						Name: "JoinServer",
 					},
-					HasContext:    true,
 					ContainerType: "MessagesContainer",
 				},
 				AsserterMethod{ChildType: "Sender"},
@@ -1251,7 +1246,6 @@ var Main = Packages{
 						Name: "Backlog",
 					},
 					Parameters: []NamedType{
-						{"ctx", "context.Context"},
 						{"before", "ID"},
 						{"msgc", "MessagesContainer"},
 					},
@@ -1278,7 +1272,6 @@ var Main = Packages{
 						`},
 						Name: "ListMembers",
 					},
-					HasContext:    true,
 					ContainerType: "MemberListContainer",
 				},
 			},