As far as I understand it this is taken care of by calling dispatch_main().
Per man dispatch_main:
Programs must call dispatch_main() at the end of main() in order to process blocks submitted to the main queue."
Two changes in test client based on internal review by XPC folks:
- Removed cleanup code at the end of main() as dispatch_main() never returns.
- Removed check for conn as xpc_connection_create() is guaranteed to succeed.
Not really. The documentation for the parameter states the following:
The GCD queue to which the event handler block will be submitted. This parameter may be NULL, in which case the connection's target queue will be libdispatch's default target queue, defined as DISPATCH_TARGET_QUEUE_DEFAULT. The target queue may be changed later with a call to xpc_connection_set_target_queue().
So the handler blocks will be submitted to the concurrent queue (DISPATCH_TARGET_QUEUE_DEFAULT) which means that they might end up running concurrently.
Just an initial couple of thoughts here, haven't yet been through in detail.
Mostly I wonder if we can use slightly different abstractions in D48559 to so the JSON/XPC parts get the code reuse we want but the work required to call one vs the other is smaller. Need to think about this more.
nit: can we give these the same name?
Maybe the latter is better, since an environment variable controls the runtime behavior anyway.
is there a reason this is an environment variable rather than an -xpc flag?
Apart from this being (mostly) what we do elsewhere, it seems like if the process spawning clangd wants XPC suppport, but clangd was built without it, then you want the process to fail and exit rather than sit there listening on stdin. Flags have this behavior, env variables do not.
the duplication here suggests to me the factoring isn't quite right.
naively, ISTM we should be able to have two implementations of an interface here rather than an actual ifdef'd server loop. But I'll dig into the implementation to try to understand a bit more.
Hi Sam, I am definitely open to discussion about the right abstraction.
I will push patches rebased on TOT and changes based on your comments today or tomorrow and I am trying to figure out how could we use your Transport abstraction proposal (https://reviews.llvm.org/D49389) without Json in it's interface in the meantime.
Good point. I will change that.
I agree that flag would be a natural choice here (and was my original intention) but unfortunately Launchd doesn't support that for spawning bundled XPC services.
Do you mean I should check for the env variable also in case clangd is not built with XPC support and conditionally exit?
I totally understand your thoughts - I originally started with the same idea in the end wasn't able to implement it in simple enough way. Ultimately I decided to prefer simplicity over nice abstraction.