Skip to content

Commit b20c35e

Browse files
authored
fix: Close the final initializer on exhaustion; document the source manager contract (#304)
## What this changes Two small behavior hardenings and an explicit statement of the source manager's API contract. - **Initializer exhaustion closes the final initializer.** Every other transition closes the previous source when the next one is activated, but the exhaustion path returned null with the last initializer still active. When no synchronizer activation follows, that initializer (and its HTTP client) was held until shutdown. A terminal null now leaves no source running. - **`engageFdv1Fallback()` is a no-op when no FDv1 fallback slot is configured.** Blocking every slot without unblocking a fallback would leave nothing to activate. The orchestration layer checks before engaging; the guard makes the operation safe regardless of the caller. - **`hasFdv1FallbackConfigured`** replaces `hasFdv1Fallback` to make clear it reports configuration, not engagement state. ## The documented contract The class docs now state the assumptions the manager operates under, so they are visible at the API surface rather than implicit in the call sites: - The manager assumes a single, sequential driver. At most one source is active at a time: activating a source closes the previous one, exhaustion closes the final initializer, and `close` closes whatever remains. - The synchronizer scan cursor doubles as the active-slot pointer. From an activation (`nextAvailableSynchronizer` / `recreateCurrentSynchronizer`) until the next cursor mutation (`resetSynchronizerIndex` / `engageFdv1Fallback`), the cursor identifies the running synchronizer's slot — `isPrimarySynchronizer` and `blockCurrentSynchronizer` read it and are only meaningful inside that window. After mutating the cursor, a new synchronizer must be activated before consulting either. The driver satisfies this by construction: both reads happen strictly between an activation and the run's outcome being decided, and cursor mutations happen only after the outcome is decided. ## Testing New tests pin the exhaustion close (a terminal null leaves no source running) and the fallback-engagement guard (engaging with no configured FDv1 slot leaves the slot list untouched). Full package suite passes. SDK-2186
1 parent 761ed9d commit b20c35e

2 files changed

Lines changed: 74 additions & 10 deletions

File tree

packages/common_client/lib/src/data_sources/fdv2/source_manager.dart

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,21 @@ final class SynchronizerSlot {
3333

3434
/// Manages the state of initializers and synchronizers, tracks which
3535
/// source is active, and handles source transitions for the orchestrator.
36+
///
37+
/// The manager assumes a single, sequential driver and relies on these
38+
/// contracts:
39+
///
40+
/// - At most one source is active at a time. Activating a source closes
41+
/// the previously active one, exhausting the initializer list closes
42+
/// the final initializer, and [close] closes whatever remains.
43+
/// - The synchronizer scan cursor doubles as the active-slot pointer:
44+
/// from an activation ([nextAvailableSynchronizer] or
45+
/// [recreateCurrentSynchronizer]) until the next cursor mutation
46+
/// ([resetSynchronizerIndex] or [engageFdv1Fallback]), the cursor
47+
/// identifies the running synchronizer's slot. [isPrimarySynchronizer]
48+
/// and [blockCurrentSynchronizer] read the cursor and are only
49+
/// meaningful inside that window. After mutating the cursor, activate
50+
/// a new synchronizer before consulting either of them.
3651
final class SourceManager {
3752
final List<InitializerFactory> _initializerFactories;
3853
final List<SynchronizerSlot> _synchronizerSlots;
@@ -66,12 +81,14 @@ final class SourceManager {
6681

6782
/// Get the next initializer and set it as the active source. Closes the
6883
/// previous active source. Returns null when all initializers are
69-
/// exhausted.
84+
/// exhausted; exhaustion also closes the final initializer, so a
85+
/// terminal null leaves no source running.
7086
Initializer? nextInitializer() {
7187
if (_shutdown) return null;
7288

7389
_initializerIndex += 1;
7490
if (_initializerIndex >= _initializerFactories.length) {
91+
_closeActiveSource();
7592
return null;
7693
}
7794

@@ -130,8 +147,10 @@ final class SourceManager {
130147
return synchronizer;
131148
}
132149

133-
/// Mark the current synchronizer as blocked (e.g. after a terminal
134-
/// error).
150+
/// Mark the active synchronizer's slot as blocked (e.g. after a
151+
/// terminal error). Reads the scan cursor, so it must only be called
152+
/// while the cursor identifies the active slot (see the class
153+
/// contract).
135154
void blockCurrentSynchronizer() {
136155
if (_synchronizerIndex >= 0 &&
137156
_synchronizerIndex < _synchronizerSlots.length) {
@@ -140,14 +159,25 @@ final class SourceManager {
140159
}
141160
}
142161

143-
/// Reset the synchronizer scan position so the next call to
144-
/// [nextAvailableSynchronizer] starts from the beginning.
162+
/// Reset the synchronizer scan cursor so the next call to
163+
/// [nextAvailableSynchronizer] starts from the beginning. After a
164+
/// reset the cursor no longer identifies the active slot; activate a
165+
/// new synchronizer before consulting [isPrimarySynchronizer] or
166+
/// [blockCurrentSynchronizer].
145167
void resetSynchronizerIndex() {
146168
_synchronizerIndex = -1;
147169
}
148170

149-
/// Block all non-FDv1 synchronizers and unblock FDv1 synchronizers.
171+
/// Block all non-FDv1 synchronizers, unblock the FDv1 fallback, and
172+
/// reset the scan cursor so the next activation selects the fallback.
173+
/// Does nothing when no FDv1 fallback slot is configured, since
174+
/// blocking every slot without unblocking one would leave nothing to
175+
/// activate. The active source keeps running until the next
176+
/// activation closes it.
150177
void engageFdv1Fallback() {
178+
if (!hasFdv1FallbackConfigured) {
179+
return;
180+
}
151181
for (final slot in _synchronizerSlots) {
152182
slot.state = slot.isFdv1Fallback
153183
? SynchronizerSlotState.available
@@ -156,7 +186,10 @@ final class SourceManager {
156186
_synchronizerIndex = -1;
157187
}
158188

159-
/// True if the current synchronizer is the first available (primary).
189+
/// True if the active synchronizer occupies the first available slot
190+
/// (the primary). Reads the scan cursor, so it is only meaningful
191+
/// while the cursor identifies the active slot (see the class
192+
/// contract).
160193
bool get isPrimarySynchronizer =>
161194
_synchronizerIndex == _firstAvailableIndex();
162195

@@ -165,8 +198,9 @@ final class SourceManager {
165198
.where((slot) => slot.state == SynchronizerSlotState.available)
166199
.length;
167200

168-
/// True if any synchronizer slot is marked as an FDv1 fallback.
169-
bool get hasFdv1Fallback =>
201+
/// True if any synchronizer slot is the FDv1 fallback, whether or not
202+
/// it has been engaged.
203+
bool get hasFdv1FallbackConfigured =>
170204
_synchronizerSlots.any((slot) => slot.isFdv1Fallback);
171205

172206
/// Close the active source and mark the manager as shut down.

packages/common_client/test/data_sources/fdv2/source_manager_test.dart

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,23 @@ void main() {
7676
reason: 'starting the second source closes the first');
7777
});
7878

79+
test('initializer exhaustion closes the final initializer', () {
80+
final created = <RecordingInitializer>[];
81+
final factory = InitializerFactory(create: (_) {
82+
final initializer = RecordingInitializer();
83+
created.add(initializer);
84+
return initializer;
85+
});
86+
87+
final manager = _manager(initializers: [factory]);
88+
89+
expect(manager.nextInitializer(), isNotNull);
90+
expect(created.single.closed, isFalse);
91+
expect(manager.nextInitializer(), isNull);
92+
expect(created.single.closed, isTrue,
93+
reason: 'a terminal null must leave no source running');
94+
});
95+
7996
test('synchronizers cycle through available slots and wrap around', () {
8097
final created = <RecordingSynchronizer>[];
8198
final manager = _manager(slots: [_slot(0, created), _slot(1, created)]);
@@ -150,7 +167,7 @@ void main() {
150167
_slot(1, fdv1Created, isFdv1Fallback: true),
151168
]);
152169

153-
expect(manager.hasFdv1Fallback, isTrue);
170+
expect(manager.hasFdv1FallbackConfigured, isTrue);
154171
expect(manager.availableSynchronizerCount, 1);
155172

156173
manager.nextAvailableSynchronizer();
@@ -165,6 +182,19 @@ void main() {
165182
reason: 'the FDv2 tier is disabled after fallback');
166183
});
167184

185+
test('engaging FDv1 fallback without a configured slot does nothing', () {
186+
final created = <RecordingSynchronizer>[];
187+
final manager = _manager(slots: [_slot(0, created), _slot(1, created)]);
188+
189+
expect(manager.hasFdv1FallbackConfigured, isFalse);
190+
manager.engageFdv1Fallback();
191+
192+
expect(manager.availableSynchronizerCount, 2,
193+
reason: 'blocking every slot without unblocking a fallback would '
194+
'leave nothing to activate');
195+
expect(manager.nextAvailableSynchronizer(), isNotNull);
196+
});
197+
168198
test('close prevents further source creation', () {
169199
final created = <RecordingSynchronizer>[];
170200
final manager = _manager(slots: [_slot(0, created)]);

0 commit comments

Comments
 (0)