Sorry for the late response. What you're saying sounds correct.
Let me write it up in slightly different words, maybe it'll help you and others.
tl;dr Assuming you're processing events in the same ChannelPipeline that you want to add a handler to with .syncOperations.addHandler you can always add handlers synchronously (i.e. instantaneously), no buffering required. Removing handlers can only be done synchronously if you're in the EventLoop and the handler to be removed cooperates.
Full story:
So in general, both adding and removing ChannelHandlers from the ChannelPipeline is asynchronous. This means that any events (including data) can flow through the ChannelPipeline between the point where you triggered the addition/removal of a handler and the point where that addition/removal is complete.
This means that to be safe in general you will need to buffer all events during the addition/removal period and 'replay' them once the addition/removal is complete.
Please note that with "in general" I mean that you could call randomChannel.pipeline.addHandler(...) so you could add a handler to any Channel from any thread. That obviously can't be a synchronous operation because we first need to get to that Channel's EventLoop. But in most cases we don't want to add/remove handlers to/from a random Channel but the one we're currently processing events for (ie. we are already on the right EventLoop) which makes things easier.
So let's focus on what we typically actually want to do: Let's say we are processing events inside a ChannelHandler and we want to add/remove handlers to/from its own pipeline. In this can we can always add handlers synchronously (i.e. instantenously) without needing to buffer using syncOperations.addHandler(). There's not much that can go wrong and the handler will be in the pipeline once the call returns (unless it throws ofc).
Handler removal is a lot more difficult. We already understand that we obviously can't remove an arbitrary handler from a random Channel synchronously (because we might not even be on the right EventLoop). But even in the easier case where we want to remove a handler from the pipeline we're processing events in, removal cannot be done synchronously. That sounds odd, after all we can add handlers. But thinking about it more, we don't necessarily know anything about the handler that we want to remove. What if that handler has buffered a large number of events? We can't possibly remove it safely because that would mean all the buffered events are lost.
Therefore, in SwiftNIO, ChannelHandler removal is opt-in (your handler needs to implement RemovableChannelHandler) and asynchronous. That means if you call removeHandler, the handler will be told to remove itself and it can take any amount of time to do so. Of course, well behaved handlers do so as soon as possible but SwiftNIO can't guarantee when this will be. That means that even when you are on the right EventLoop, you'll need to buffer all events during the removal period.
Again, this sounds annoying and it is. But in many cases there's a way out which isn't pretty but works assuming you want to remove a handler that you know a little more about, e.g. you wrote that handler.
You might know that the handler you'd like to remove doesn't actually buffer anything and it will call leavePipeline immediately.
If that's the case, you can write code like below which _precondition_s that the removal happened synchronously which is fine if you know that the to-be-removed handler calls leavePipeline synchronously.
func channelRead(...) {
[...]
// we can `try!` this because we know that we can add after ourselves
try! context.pipeline.syncOperations.addHandler(NewHandler, position: .after(self))
var isSynchronous = true
defer {
isSynchronous = false // set to false at the end of this function
}
context.pipeline.removeHandler(someOtherHandlerWeKnowIsIn)
.recover { error in
preconditionFailure("handler isn't actually in")
}.whenSuccess {
precondition(isSynchronous) // we assert that `isSynchronous ` is still true when it completes which is only the case if it completes synchronously
}
}
That sets a precondition on the removal actually being synchronous. I know this isn't pretty but as explained above, we can only synchronously remove a handler if it cooperates. So this essentially preconditons on the cooperation of the other handler which must guarantee that it can be synchronously removed in any situation. This is usually only easily doable if it doesn't buffer anything.
FWIW, if you just write class MyHandler: RemovableChannelHandler and you don't implement removeHandler(context:removalToken:) you'll get the default implementation which calls leavePipeline synchronously. So a handler which doesn't buffer can just do that and then (if on the pipeline) you can just assume synchronous removal.
A word of warning: I think it's super great that SwiftNIO does support mutating the ChannelPipeline at runtime. Many real-world network protocols become much easier and faster to implement with this. Examples are protocol upgrades/downgrades such as WebSocket, STARTTLS, STOPTLS, ...
But: pipeline mutations are one of the easiest ways to shoot yourself in the foot. So I would very much only recommend using them if there's no other, better solution. It's fine for a protocol upgrade, ie. you start without TLS but then add TLS (fine, just one handler addition which can be done synchronously).
It's also fine if it's a more complicated protocol upgrade and you may need to implement some buffering. And most importantly, if you do pipeline mutations you'll need to write an awful lot of unit tests, understand pipeline re-entrancy and have a good understanding of all the other, less used events that flow through the pipeline. Also edge cases like what happens if a channel fails/closes during the removal period, are you handling that okay?
