Revert "Replace stop callbacks with contexts"

This reverts commit 410ac73469.

The rationale is that the frontend can wrap its own components with a
disposable thread-safe wrapper that doesn't work once it's invalidated
if it wants the guarantee that the component doesn't work anymore once
the context is stopped.

cchat.go

@@ -355,7 +355,7 @@ type Identifier interface {
 // Labels given to the frontend may contain images or avatars, and the frontend
 // has the choice to display them or not.
 type LabelContainer interface {
-	SetLabel(text.Rich)
+	SetLabel(context.Context, text.Rich)
 }
 
 // ListMember represents a single member in the member list. Note that this
@@ -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(context.Context, ServersContainer) error
+	Servers(ServersContainer) (stop func(), err 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
@@ -452,7 +452,7 @@ type MemberDynamicSection interface {
 type MemberListContainer interface {
 	// RemoveMember removes a member from a section. If neither the member nor the
 	// section exists, then the client should ignore it.
-	RemoveMember(sectionID ID, memberID ID)
+	RemoveMember(ctx context.Context, sectionID ID, memberID ID)
 	// SetMember adds or updates (or upsert) a member into a section. This operation
 	// must not change the section's member count. As such, changes should be done
 	// separately in SetSection. If the section does not exist, then the client
@@ -462,12 +462,12 @@ type MemberListContainer interface {
 	// Typically, the backend should try and avoid calling this method and instead
 	// update the labeler in the name. This method should only be used for adding
 	// members.
-	SetMember(sectionID ID, member ListMember)
+	SetMember(ctx context.Context, sectionID ID, member ListMember)
 	// SetSections (re)sets the list of sections to be the given slice. Members from
 	// the old section list should be transferred over to the new section entry if
 	// the section name's content is the same. Old sections that don't appear in the
 	// new slice should be removed.
-	SetSections(sections []MemberSection)
+	SetSections(ctx context.Context, sections []MemberSection)
 }
 
 // MemberLister adds a member list into a message server.
@@ -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) error
+	ListMembers(context.Context, MemberListContainer) (stop func(), err error)
 }
 
 // MemberSection represents a member list section. The section name's content
@@ -537,12 +537,12 @@ type MessageUpdate interface {
 // allowed to have multiple views. This is usually done with tabs or splits, but
 // the backend should update them all nonetheless.
 type MessagesContainer interface {
-	DeleteMessage(MessageDelete)
-	UpdateMessage(MessageUpdate)
+	DeleteMessage(context.Context, MessageDelete)
+	UpdateMessage(context.Context, MessageUpdate)
 	// CreateMessage inserts a message into the container. The frontend must
 	// guarantee that the messages are in order based on what's returned from
 	// Time().
-	CreateMessage(MessageCreate)
+	CreateMessage(context.Context, MessageCreate)
 }
 
 // Messenger is for servers that contain messages. This is similar to Discord or
@@ -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) error
+	JoinServer(context.Context, MessagesContainer) (stop func(), err 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) error
+	Name(context.Context, LabelContainer) (stop func(), err error)
 }
 
 // Nicknamer adds the current user's nickname.
@@ -618,10 +618,10 @@ type ReadContainer interface {
 	// their read indicators. The backend can use this to free up users/authors that
 	// are no longer in the server, for example when they are offline or have left
 	// the server.
-	DeleteIndications(authorIDs []ID)
+	DeleteIndications(ctx context.Context, authorIDs []ID)
 	// AddIndications adds a map of users/authors to the respective message ID of
 	// the server that implements ReadIndicator.
-	AddIndications([]ReadIndication)
+	AddIndications(context.Context, []ReadIndication)
 }
 
 // ReadIndicator adds a read indicator API for frontends to show. An example of
@@ -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) error
+	ReadIndicate(context.Context, ReadContainer) (stop func(), err error)
 }
 
 // Replier indicates that the message being sent is a reply to something.
@@ -721,7 +721,7 @@ type ServerUpdate interface {
 // as servers can be infinitely nested. Frontends should also reset the entire
 // node and its children when SetServers is called again.
 type ServersContainer interface {
-	UpdateServer(ServerUpdate)
+	UpdateServer(context.Context, ServerUpdate)
 	// SetServer is called by the backend service to request a reset of the server
 	// list. The frontend can choose to call Servers() on each of the given servers,
 	// or it can call that later. The backend should handle both cases.
@@ -731,7 +731,7 @@ type ServersContainer interface {
 	// should only be considered empty if it's an empty non-nil slice. An
 	// unavailable list, on the other hand, can be treated as backend issues, e.g. a
 	// connection issue.
-	SetServers([]Server)
+	SetServers(context.Context, []Server)
 }
 
 // Service is a complete service that's capable of multiple sessions. It has to
@@ -835,11 +835,11 @@ type TypingContainer interface {
 	// of typers. This function is usually not needed, as the client will take care
 	// of removing them after TypingTimeout has been reached or other conditions
 	// listed in ServerMessageTypingIndicator are met.
-	RemoveTyper(authorID ID)
+	RemoveTyper(ctx context.Context, authorID ID)
 	// AddTyper appends the typer (author) into the frontend's list of typers, or it
 	// pushes this typer on top of others. The frontend should assume current time
 	// every time AddTyper is called.
-	AddTyper(User)
+	AddTyper(context.Context, User)
 }
 
 // TypingIndicator optionally extends ServerMessage to provide bidirectional
@@ -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) error
+	TypingSubscribe(context.Context, TypingContainer) (stop func(), err 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.
@@ -885,7 +885,7 @@ type TypingIndicator interface {
 type UnreadContainer interface {
 	// SetUnread sets the container's unread state to the given boolean. The
 	// frontend may choose how to represent this.
-	SetUnread(unread bool, mentioned bool)
+	SetUnread(ctx context.Context, unread bool, mentioned bool)
 }
 
 // UnreadIndicator adds an unread state API for frontends to use. The unread
@@ -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) error
+	UnreadIndicate(context.Context, UnreadContainer) (stop func(), err 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.
@@ -908,7 +908,7 @@ type UnreadIndicator interface {
 	// the frontend has no use in knowing the error. As such, marking messages as
 	// read is best-effort. The backend is in charge of synchronizing the read state
 	// with the server and coordinating it with reasonable rate limits, if needed.
-	MarkRead(messageID ID)
+	MarkRead(ctx context.Context, messageID ID)
 }
 
 // User is the interface for an identifiable author. The interface defines that

cmd/internal/cchat-generator/generate_interface.go

@@ -67,6 +67,9 @@ func generateInterfaces(ifaces []repository.Interface) jen.Code {
 				case repository.SetterMethod:
 					stmt.Params(generateFuncParams(method.Parameters, "")...)
 					stmt.Params(generateFuncParamsErr(repository.NamedType{}, method.ErrorType)...)
+				case repository.ContainerUpdaterMethod:
+					stmt.Params(generateFuncParamsCtx(method.Parameters, "")...)
+					stmt.Params(generateFuncParamsErr(repository.NamedType{}, method.ErrorType)...)
 				case repository.IOMethod:
 					stmt.Params(generateFuncParamsCtx(method.Parameters, "")...)
 					stmt.Params(generateFuncParamsErr(method.ReturnValue, method.ErrorType)...)
@@ -156,13 +159,20 @@ func generateFuncParams(params []repository.NamedType, errorType string) []jen.C
 }
 
 func generateContainerFuncReturns(method repository.ContainerMethod) []jen.Code {
-	return []jen.Code{jen.Error()}
+	var stmt jen.Statement
+
+	stmt.Add(jen.Id("stop").Func().Params())
+	stmt.Add(jen.Err().Error())
+
+	return stmt
 }
 
 func generateContainerFuncParams(method repository.ContainerMethod) []jen.Code {
 	var stmt jen.Statement
 
-	stmt.Qual("context", "Context")
+	if method.HasContext {
+		stmt.Qual("context", "Context")
+	}
 	stmt.Add(genutils.GenerateType(method))
 
 	return stmt

generator.go

@@ -1,7 +1,5 @@
 package cchat
 
-import "context"
-
 //go:generate go run ./cmd/internal/cchat-generator ./
 //go:generate go run ./cmd/internal/cchat-empty-gen ./utils/empty/
 
@@ -18,14 +16,3 @@ 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

@@ -10,6 +10,7 @@ func init() {
 	gob.Register(AsserterMethod{})
 	gob.Register(GetterMethod{})
 	gob.Register(SetterMethod{})
+	gob.Register(ContainerUpdaterMethod{})
 	gob.Register(IOMethod{})
 }
 
@@ -67,11 +68,7 @@ func (m GetterMethod) ReturnError() bool {
 }
 
 // SetterMethod is a method that sets values. These methods must not do IO, and
-// they have to be non-blocking. They're used only for containers. Actual setter
-// methods implemented by the backend belongs to IOMethods.
-//
-// Clients should always keep track of the values given to setters and free them
-// once the setters are called again with new values.
+// they have to be non-blocking.
 type SetterMethod struct {
 	method
 
@@ -84,6 +81,22 @@ type SetterMethod struct {
 	ErrorType string
 }
 
+// ContainerUpdaterMethod is a SetterMethod that passes to the container the
+// current context to prevent race conditions when synchronizing.
+// The rule of thumb is that any setter method done inside a method with a
+// context is usually this type of method.
+type ContainerUpdaterMethod struct {
+	method
+
+	// 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. IOMethods must always have means of
 // cancelling them in the API, but implementations don't have to use it; as
@@ -117,13 +130,13 @@ func (m IOMethod) ReturnError() bool {
 	return m.ErrorType != ""
 }
 
-// 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.
+// ContainerMethod is a method that uses a Container. These methods can do IO
+// and always return a stop callback and an error.
 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,6 +603,7 @@ var Main = Packages{
 						`},
 						Name: "Name",
 					},
+					HasContext:    true,
 					ContainerType: "LabelContainer",
 				},
 			},
@@ -1068,6 +1069,7 @@ var Main = Packages{
 						`},
 						Name: "JoinServer",
 					},
+					HasContext:    true,
 					ContainerType: "MessagesContainer",
 				},
 				AsserterMethod{ChildType: "Sender"},
@@ -1272,6 +1274,7 @@ var Main = Packages{
 						`},
 						Name: "ListMembers",
 					},
+					HasContext:    true,
 					ContainerType: "MemberListContainer",
 				},
 			},
@@ -1295,6 +1298,7 @@ var Main = Packages{
 						`},
 						Name: "ReadIndicate",
 					},
+					HasContext:    true,
 					ContainerType: "ReadContainer",
 				},
 			},
@@ -1307,7 +1311,7 @@ var Main = Packages{
 			`},
 			Name: "UnreadIndicator",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							MarkRead marks a message in the server messenger as
@@ -1341,6 +1345,7 @@ var Main = Packages{
 						`},
 						Name: "UnreadIndicate",
 					},
+					HasContext:    true,
 					ContainerType: "UnreadContainer",
 				},
 			},
@@ -1396,6 +1401,7 @@ var Main = Packages{
 						`},
 						Name: "TypingSubscribe",
 					},
+					HasContext:    true,
 					ContainerType: "TypingContainer",
 				},
 			},
@@ -1446,7 +1452,7 @@ var Main = Packages{
 			`},
 			Name: "ServersContainer",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							SetServer is called by the backend service to
@@ -1467,7 +1473,7 @@ var Main = Packages{
 					},
 					Parameters: []NamedType{{Type: "[]Server"}},
 				},
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method:     method{Name: "UpdateServer"},
 					Parameters: []NamedType{{Type: "ServerUpdate"}},
 				},
@@ -1527,7 +1533,7 @@ var Main = Packages{
 			`},
 			Name: "MessagesContainer",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							CreateMessage inserts a message into the container.
@@ -1538,11 +1544,11 @@ var Main = Packages{
 					},
 					Parameters: []NamedType{{Type: "MessageCreate"}},
 				},
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method:     method{Name: "UpdateMessage"},
 					Parameters: []NamedType{{Type: "MessageUpdate"}},
 				},
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method:     method{Name: "DeleteMessage"},
 					Parameters: []NamedType{{Type: "MessageDelete"}},
 				},
@@ -1631,7 +1637,7 @@ var Main = Packages{
 			`},
 			Name: "LabelContainer",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{Name: "SetLabel"},
 					Parameters: []NamedType{{
 						Type: MakeQual("text", "Rich"),
@@ -1647,7 +1653,7 @@ var Main = Packages{
 			`},
 			Name: "ReadContainer",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							AddIndications adds a map of users/authors to the
@@ -1658,7 +1664,7 @@ var Main = Packages{
 					},
 					Parameters: []NamedType{{"", "[]ReadIndication"}},
 				},
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							DeleteIndications deletes a list of unused
@@ -1692,7 +1698,7 @@ var Main = Packages{
 			`},
 			Name: "UnreadContainer",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							SetUnread sets the container's unread state to the
@@ -1718,7 +1724,7 @@ var Main = Packages{
 			`},
 			Name: "TypingContainer",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							AddTyper appends the typer (author) into the
@@ -1730,7 +1736,7 @@ var Main = Packages{
 					},
 					Parameters: []NamedType{{"", "User"}},
 				},
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							RemoveTyper explicitly removes the typer with the
@@ -1770,7 +1776,7 @@ var Main = Packages{
 			`},
 			Name: "MemberListContainer",
 			Methods: []Method{
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							SetSections (re)sets the list of sections to be the
@@ -1786,7 +1792,7 @@ var Main = Packages{
 						{Name: "sections", Type: "[]MemberSection"},
 					},
 				},
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							SetMember adds or updates (or upsert) a member into
@@ -1809,7 +1815,7 @@ var Main = Packages{
 						{"member", "ListMember"},
 					},
 				},
-				SetterMethod{
+				ContainerUpdaterMethod{
 					method: method{
 						Comment: Comment{`
 							RemoveMember removes a member from a section. If