make extension activation transactional (T-377)
Three lifecycle gaps, benign among curated builtins but hazardous the day Tier-6 Lua extensions land: a throw mid-contribution left earlier contributions mounted while the extension recorded as failed (a retry then double-applied them); deactivate ignored active dependents; and the panel/command registries silently clobbered on id collision. Activation now tracks what it mounted and unwinds it all on failure (including the extension's own deactivate when its activate had succeeded); deactivate refuses with a logged warning while active dependents exist — disable the dependents first; duplicate contribution/command ids throw, which the transactional path turns into a clean failed activation with first-wins semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -144,10 +144,17 @@ class ExtensionManager extends ChangeNotifier {
|
||||
}
|
||||
}
|
||||
final ctx = _ExtensionContext(manager: this, id: ext.id);
|
||||
// Transactional: a throw mid-activation must leave NOTHING mounted —
|
||||
// the old path left earlier contributions live while the extension
|
||||
// recorded as failed, and a retry double-applied them (T-377).
|
||||
final applied = <ContributionPoint>[];
|
||||
var extActivated = false;
|
||||
try {
|
||||
await ext.activate(ctx);
|
||||
extActivated = true;
|
||||
for (final c in ext.contributions) {
|
||||
_applyContribution(c);
|
||||
applied.add(c);
|
||||
}
|
||||
// Eagerly load the i18n catalog for any localized tab this extension
|
||||
// contributes, so its title resolves without a "namespace not
|
||||
@@ -165,6 +172,22 @@ class ExtensionManager extends ChangeNotifier {
|
||||
notifyListeners();
|
||||
log.info('extensions', 'activated $id');
|
||||
} catch (e, st) {
|
||||
for (final c in applied.reversed) {
|
||||
try {
|
||||
_removeContribution(c);
|
||||
} catch (e2) {
|
||||
log.warn('extensions', 'unwind of ${c.id} failed during $id rollback: $e2');
|
||||
}
|
||||
}
|
||||
if (extActivated) {
|
||||
// The extension's own activate() succeeded — give it the matching
|
||||
// teardown so it doesn't hold resources for a failed activation.
|
||||
try {
|
||||
await ext.deactivate();
|
||||
} catch (e2) {
|
||||
log.warn('extensions', 'deactivate during $id rollback failed: $e2');
|
||||
}
|
||||
}
|
||||
_failed[id] = e;
|
||||
log.error('extensions', 'activate failed for $id', error: e, stackTrace: st);
|
||||
notifyListeners();
|
||||
@@ -175,6 +198,17 @@ class ExtensionManager extends ChangeNotifier {
|
||||
if (!_activated.contains(id)) return;
|
||||
final ext = _known[id];
|
||||
if (ext == null) return;
|
||||
// Refuse while active extensions depend on this one — deactivating
|
||||
// underneath them leaves them running against missing services (T-377).
|
||||
// Disable the dependents first.
|
||||
final dependents = [
|
||||
for (final e in _known.values)
|
||||
if (_activated.contains(e.id) && e.dependsOn.contains(id)) e.id,
|
||||
];
|
||||
if (dependents.isNotEmpty) {
|
||||
log.warn('extensions', 'refusing to deactivate $id: active dependents: ${dependents.join(', ')}');
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await ext.deactivate();
|
||||
for (final c in ext.contributions) {
|
||||
@@ -196,8 +230,17 @@ class ExtensionManager extends ChangeNotifier {
|
||||
case TabContribution _:
|
||||
case StatusItemContribution _:
|
||||
case ToolbarButtonContribution _:
|
||||
// Reject duplicates instead of silently mounting a second copy —
|
||||
// benign among curated builtins, hazardous once third-party
|
||||
// extensions land (T-377). The throw rolls the activation back.
|
||||
if (panels.hasContribution(c.id)) {
|
||||
throw StateError('duplicate contribution id: ${c.id}');
|
||||
}
|
||||
panels.contribute(c);
|
||||
case CommandContribution cmd:
|
||||
if (commands.get(cmd.command) != null) {
|
||||
throw StateError('duplicate command id: ${cmd.command}');
|
||||
}
|
||||
commands.register(cmd);
|
||||
final binding = cmd.defaultBinding;
|
||||
if (binding != null) {
|
||||
|
||||
@@ -30,6 +30,11 @@ class PanelRegistry extends ChangeNotifier {
|
||||
notifyListeners();
|
||||
}
|
||||
|
||||
/// Whether any slot already mounts a contribution with [id]. Used by the
|
||||
/// extension manager to reject duplicate ids instead of silently mounting
|
||||
/// a second copy (T-377).
|
||||
bool hasContribution(String id) => _mounts.values.any((list) => list.any((c) => c.id == id));
|
||||
|
||||
void contribute(ContributionPoint point) {
|
||||
final slot = point.slot;
|
||||
if (slot == null) return;
|
||||
|
||||
Reference in New Issue
Block a user