trilemma-dev / SecureXPC

A simple and secure XPC framework for Swift
MIT License
75 stars 15 forks source link

Proper server deallocation, fixes issue #54 #56

Closed jakaplan closed 2 years ago

jakaplan commented 2 years ago

XPCAnonymousServer and XPCMachServer now both start their listener connections in their initializers. However, now when incoming connections are received they don't unconditionally configure and start them. If the server has not been started, the connections are added to a pending array. Once the server is started, those pending connections are configured and started.

Summary of changes:

jakaplan commented 2 years ago

@amomchilov for your review

FYI I'll be away for the next three days, probably will have pretty spotty Internet access.

amomchilov commented 2 years ago

There's a lot of different changes here, and I'm struggling to understand how they're related.

If the server has not been started, the connections are added to a pending array. Once the server is started, those pending connections are configured and started.

Could you please elaborate on the motivation for this queuing mechanic?

jakaplan commented 2 years ago

If the server has not been started, the connections are added to a pending array. Once the server is started, those pending connections are configured and started.

Could you please elaborate on the motivation for this queuing mechanic?

The listener connection needs to be started in the server's initializer to avoid misuse of the XPC API. This means client connections can now be received by the listener connection. However, we don't want to start receiving events from those client connections yet. By storing the pending connections in an array, then once the server is started they can all be activated (resumed) which will result in processing their events.

amomchilov commented 2 years ago

to avoid misuse of the XPC API

Ah was this because deallocated a connection that wasn't started causes an error?

Makes sense.

However, we don't want to start receiving events from those client connections yet.

Devil's advocate: why not? What if we rolled the starting functionality into the initializer?

jakaplan commented 2 years ago

Ah was this because deallocated a connection that wasn't started causes an error?

Indeed, I tried to document this in some detail in issue #54. Turns out this misuse actually is documented by Apple.

However, we don't want to start receiving events from those client connections yet.

Devil's advocate: why not? What if we rolled the starting functionality into the initializer?

If we rolled starting functionality into the initializer it would mean we'd have to also roll registering routes into the initializer as well. Routes need to be registered before the server is started because at least one incoming event will be received immediately upon resuming (aka starting) the connection as that's what cause the connection to be received by the listener connection.

Additionally, this would be rather weird for XPC Services as the only way to start them is in a blocking manner so the initializer itself would never return.