Provider lifecycle race condition in multi-threaded SDKs

[dd-oleksii]

I've been digging provider lifecycle in Go and Swift SDKs, and I believe there's a couple of things in OF spec that make it hard to implement it correctly in the presence of true multi-threading.

The main one is that there's no clear ownership of the provider state and provider events — it's split between the provider and the API, and both provider and the API are allowed to emit events. This is an issue because neither the provider nor the API are equipped to ensure the consistent ordering of events

The API is required to emit "provider ready" event and update the state after initialization is complete. However, once the provider returns from the init function, there's a brief window before the API updates the state and emits the event. This is not an issue in single-threaded languages like js or python. However, with multi-threading, the provider is allowed to do work in the background and emit an error event within that window — and now the order of provider ready and error events is undefined.

Current state across multi-threaded SDKs:

• Go: race condition
• Swift: 🐛 implements eventing but does not update state on provider events
• Java: race condition
• Kotlin: race condition
• Dotnet: race condition
• Rust: eventing not implemented

This would be trivial to fix if only one of API or provider were allowed to emit events (or update the state), so they could use locks/queues to order the events/updates properly.

If both are allowed to emit, the only way to prevent a race is for the API to guard eventing with a lock and to hold this lock throughout the provider initialization. This way, no provider events would come through until API processes its own ready event first.

The main issue with this approach is that it locks up the API/provider and serializes all calls to init/shutdown/on context change. This more or less completely destroys concurrency and prevents the provider from reacting to context change / shutdown if initialization is taking a while. This also easily leads to deadlocks if provider tries to emit events from/during lifecycle handlers.

Both race condition and total locking sound bad, so I would really suggest moving state management responsibility back to providers — they are the only party who knows their own state at all times, and they are equipped to resolve any races.

I see that the spec is trying to lift the burden from provider authors but imo SDK trying to manage providers' state is not helping. Anecdotally, I can add that as a provider author, the eventing and state management has been the most confusing part of the OF spec/SDKs, and I've spent quite a bit of time trying to make SDK set the state I want.

[toddbaert]

Very insightfull summary @dd-oleksii .

The API is required to emit "provider ready" event and update the state after initialization is complete. However, once the provider returns from the init function, there's a brief window before the API updates the state and emits the event. This is not an issue in single-threaded languages like js or python. However, with multi-threading, the provider is allowed to do work in the background and emit an error event within that window — and now the order of provider ready and error events is undefined.

This is the key point.

It's tough, because having the state in the provider was a foot gun. It would certainly would solve this race, but it forces an stateful abstraction onto providers, which are currently an interface that can be assumed to be stateless. I would love to find a way to solve this without moving state back into the providers.

cc @kinyoklion @lukas-reining

[kinyoklion]

It would certainly would solve this race, but it forces an stateful abstraction onto providers, which are currently an interface that can be assumed to be stateless.

I do feel like that may be a mismatch in what providers do, and the abstraction we are using. As almost all providers do have relevant state, some of them very simple and some of them very complex. It is reasonably easy to translate the state upon request, but potentially more complex to derive the state through observation. The exceptions would be in-memory (or hard-coded) providers that are ready for use upon construction. Or providers that block the thread doing their construction until they have finished their initialization (which do have state, but largely bypass this problem, and is the default behavior for most of the LaunchDarkly providers which would be affected by this issue).

I think we could infer the state based on events from providers, but inferring the state from a combination of data sources, makes this harder to keep safe. Currently the combination basically being a function call completing + events. It does still depend on the provider emitting its events in a way that is deterministic regardless of which threads get priority.

[toddbaert]

I believe this is one of those situations where it's worth calling a technical committee meeting (anyone else interested can also attend). I suggest we spend some time this week thinking about solutions, and I will create a meeting poll and we can come to a decision. As I see it, we have 2 options:

• investigate what can be done to address this race in multi-threaded SDKs, with locks or some kind of queue
• move the state back to the providers
  • if this is the best choice, we need to understand the migration/compatibility strategy

[toddbaert]

Meeting poll: https://calendly.com/d/cxrk-x3h-t7r/provider-lifecycle-race-condition-in-multi-threaded-sdk

[dd-oleksii]

UPD: One technical way to solve the race I discovered in open-feature/swift-sdk#108 is asking providers to execute a callback on initialize and on context change instead of returning their result from the function. This makes it possible for providers to avoid the race by synchronizing callbacks with event emission (so they don't happen concurrently). This requires that OF SDK reacts to event emission synchronously — this is true in swift but may not be true in languages that implement events via a channel. This API is also clumsy: providers might forget to execute the callback, and it's not obvious that callbacks and events must be synchronized.