From 25e595f48f1ac287c3b4e974e4a2ec442ef4f570 Mon Sep 17 00:00:00 2001 From: Shigma Date: Thu, 2 May 2024 00:05:20 +0800 Subject: [PATCH] fix(loader): fix isolate garbage collection --- packages/loader/src/entry.ts | 42 ++++++++---- packages/loader/src/shared.ts | 28 ++++++-- packages/loader/tests/isolate.spec.ts | 92 +++++++++++++++------------ packages/loader/tests/utils.ts | 1 + 4 files changed, 102 insertions(+), 61 deletions(-) diff --git a/packages/loader/src/entry.ts b/packages/loader/src/entry.ts index 4237470..dd51b99 100644 --- a/packages/loader/src/entry.ts +++ b/packages/loader/src/entry.ts @@ -9,7 +9,7 @@ export namespace Entry { config?: any disabled?: boolean | null intercept?: Dict | null - isolate?: Dict + isolate?: Dict when?: any } } @@ -54,20 +54,30 @@ export class Entry { if (index >= 0) config.splice(index, 1) } - amend(ctx?: Context) { + resolveRealm(label: string | true) { + if (label === true) { + return '#' + this.options.id + } else { + return '@' + label + } + } + + patch(ctx?: Context, legacy?: Entry.Options) { ctx ??= this.parent.extend({ [Context.intercept]: Object.create(this.parent[Context.intercept]), [Context.isolate]: Object.create(this.parent[Context.isolate]), }) - ctx.emit('loader/patch', this) + ctx.emit('loader/patch', this, legacy) swap(ctx[Context.intercept], this.options.intercept) const newMap: Dict = Object.create(Object.getPrototypeOf(ctx[Context.isolate])) for (const [key, label] of Object.entries(this.options.isolate ?? {})) { - if (typeof label === 'string') { - newMap[key] = (this.loader.realms[label] ??= Object.create(null))[key] ??= Symbol(`${key}@${label}`) - } else if (label) { - newMap[key] = Symbol(`${key}#${this.options.id}`) - } + const realm = this.resolveRealm(label) + newMap[key] = (this.loader.realms[realm] ??= Object.create(null))[key] ??= Symbol(`${key}${realm}`) + } + for (const key in legacy?.isolate ?? {}) { + if (this.options.isolate?.[key] === legacy!.isolate![key]) continue + const name = this.resolveRealm(legacy!.isolate![key]) + this.loader._clearRealm(key, name) } const delimiter = Symbol('delimiter') ctx[delimiter] = true @@ -98,7 +108,7 @@ export class Entry { } swap(ctx[Context.isolate], newMap) for (const [, symbol1, symbol2, flag] of diff) { - if (flag && ctx[symbol1]) { + if (flag && ctx[symbol1] && !ctx[symbol2]) { ctx.root[symbol2] = ctx.root[symbol1] delete ctx.root[symbol1] } @@ -116,21 +126,21 @@ export class Entry { return ctx } - // TODO: handle parent change async update(parent: Context, options: Entry.Options) { + const legacy = this.options this.parent = parent this.options = sortKeys(options) if (!this.loader.isTruthyLike(options.when) || options.disabled) { this.stop() } else if (this.fork) { this.isUpdate = true - this.amend(this.fork.parent) + this.patch(this.fork.parent, legacy) this.fork.update(this.options.config) } else { this.parent.emit('loader/entry', 'apply', this) const plugin = await this.loader.resolve(this.options.name) if (!plugin) return - const ctx = this.amend() + const ctx = this.patch() this.fork = ctx.plugin(plugin, this.options.config) this.fork.entry = this } @@ -139,7 +149,11 @@ export class Entry { stop() { this.fork?.dispose() this.fork = undefined + + // realm garbage collection + for (const [key, label] of Object.entries(this.options.isolate ?? {})) { + const name = this.resolveRealm(label) + this.loader._clearRealm(key, name) + } } } - -Error.stackTraceLimit = 100 diff --git a/packages/loader/src/shared.ts b/packages/loader/src/shared.ts index 78916d2..be6b20d 100644 --- a/packages/loader/src/shared.ts +++ b/packages/loader/src/shared.ts @@ -11,7 +11,7 @@ declare module '@cordisjs/core' { 'config'(): void 'exit'(signal: NodeJS.Signals): Promise 'loader/entry'(type: string, entry: Entry): void - 'loader/patch'(entry: Entry): void + 'loader/patch'(entry: Entry, legacy?: Entry.Options): void } interface Context { @@ -81,7 +81,7 @@ export abstract class Loader extends super(app, 'loader', true) this.root = new Entry(this) this.entries[''] = this.root - this.realms.root = app.root[Context.isolate] + this.realms['#'] = app.root[Context.isolate] this.app.on('dispose', () => { this.exit() @@ -241,18 +241,19 @@ export abstract class Loader extends return id } - async update(id: string, options: Entry.Options) { + async update(id: string, options: Partial>) { const entry = this.entries[id] if (!entry) throw new Error(`entry ${id} not found`) + const override = { ...entry.options } for (const [key, value] of Object.entries(options)) { if (isNullable(value)) { - delete entry.options[key] + delete override[key] } else { - entry.options[key] = value + override[key] = value } } this.writeConfig() - return entry.update(entry.parent, entry.options) + return entry.update(entry.parent, override) } async create(options: Omit, target = '', index = Infinity) { @@ -313,7 +314,6 @@ export abstract class Loader extends name: 'cordis/group', config: this.config, }) - this.app.emit('config') while (this.tasks.size) { await Promise.all(this.tasks) @@ -325,6 +325,20 @@ export abstract class Loader extends } exit() {} + + _clearRealm(key: string, name: string) { + const hasRef = Object.values(this.entries).some((entry) => { + if (!entry.fork) return false + const label = entry.options.isolate?.[key] + if (!label) return false + return name === entry.resolveRealm(label) + }) + if (hasRef) return + delete this.realms[name][key] + if (!Object.keys(this.realms[name]).length) { + delete this.realms[name] + } + } } export const kGroup = Symbol.for('cordis.group') diff --git a/packages/loader/tests/isolate.spec.ts b/packages/loader/tests/isolate.spec.ts index 9ae8eda..9f311ab 100644 --- a/packages/loader/tests/isolate.spec.ts +++ b/packages/loader/tests/isolate.spec.ts @@ -7,26 +7,31 @@ import MockLoader from './utils' describe('service isolation: basic', async () => { const root = new Context() root.plugin(MockLoader) + const loader = root.loader const dispose = mock.fn() - const foo = root.loader.mock('foo', defineProperty((ctx: Context) => { + const foo = loader.mock('foo', defineProperty((ctx: Context) => { ctx.on('dispose', dispose) }, 'inject', ['bar'])) - const Bar = root.loader.mock('bar', class Bar extends Service { + const Bar = loader.mock('bar', class Bar extends Service { static [Service.provide] = 'bar' static [Service.immediate] = true }) + before(() => loader.start()) + it('initiate', async () => { - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', }, { id: '3', name: 'foo', }]) + + await new Promise((resolve) => setTimeout(resolve, 0)) expect(root.registry.get(foo)).to.be.ok expect(root.registry.get(Bar)).to.be.ok expect(foo.mock.calls).to.have.length(1) @@ -36,7 +41,7 @@ describe('service isolation: basic', async () => { it('add isolate on injector (relavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', }, { @@ -55,7 +60,7 @@ describe('service isolation: basic', async () => { it('add isolate on injector (irrelavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', }, { @@ -75,7 +80,7 @@ describe('service isolation: basic', async () => { it('remove isolate on injector (relavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', }, { @@ -94,7 +99,7 @@ describe('service isolation: basic', async () => { it('remove isolate on injector (irrelavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', }, { @@ -110,7 +115,7 @@ describe('service isolation: basic', async () => { it('add isolate on provider (relavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', isolate: { @@ -129,7 +134,7 @@ describe('service isolation: basic', async () => { it('add isolate on provider (irrelavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', isolate: { @@ -149,7 +154,7 @@ describe('service isolation: basic', async () => { it('remove isolate on provider (relavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', }, { @@ -165,7 +170,7 @@ describe('service isolation: basic', async () => { it('remove isolate on provider (irrelavent)', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - await root.loader.restart([{ + loader.root.fork!.update([{ id: '1', name: 'bar', }, { @@ -182,35 +187,26 @@ describe('service isolation: basic', async () => { describe('service isolation: realm', async () => { const root = new Context() root.plugin(MockLoader) + const loader = root.loader const dispose = mock.fn(() => { console.log(new Error()) }) - const foo = Object.assign(root.loader.mock('foo', (ctx: Context) => { + const foo = Object.assign(loader.mock('foo', (ctx: Context) => { ctx.on('dispose', dispose) }), { inject: ['bar'], reusable: true, }) - const bar = Object.assign(root.loader.mock('bar', (ctx: Context, config: {}) => { + const bar = Object.assign(loader.mock('bar', (ctx: Context, config: {}) => { ctx.set('bar', config) }), { reusable: true, }) - it('initiate', async () => { - await root.loader.restart([{ - id: '1', - name: 'foo', - }, { - id: '2', - name: 'bar', - }]) - expect(foo.mock.calls).to.have.length(1) - expect(dispose.mock.calls).to.have.length(0) - }) + before(() => loader.start()) let alpha!: string let beta!: string @@ -219,10 +215,10 @@ describe('service isolation: realm', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - alpha = await root.loader.create({ + alpha = await loader.create({ name: 'cordis/group', isolate: { - bar: 'alpha', + bar: true, }, config: [{ name: 'bar', @@ -230,7 +226,7 @@ describe('service isolation: realm', async () => { }], }) - beta = await root.loader.create({ + beta = await loader.create({ name: 'cordis/group', isolate: { bar: 'beta', @@ -242,7 +238,23 @@ describe('service isolation: realm', async () => { }) await new Promise((resolve) => setTimeout(resolve, 0)) - expect(root.registry.get(bar)?.children).to.have.length(3) + expect(root.registry.get(bar)?.children).to.have.length(2) + expect(foo.mock.calls).to.have.length(0) + expect(dispose.mock.calls).to.have.length(0) + }) + + it('update isolate group (no change)', async () => { + foo.mock.resetCalls() + dispose.mock.resetCalls() + + await loader.update(alpha, { + isolate: { + bar: true, + }, + }) + + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(root.registry.get(bar)?.children).to.have.length(2) expect(foo.mock.calls).to.have.length(0) expect(dispose.mock.calls).to.have.length(0) }) @@ -255,18 +267,18 @@ describe('service isolation: realm', async () => { foo.mock.resetCalls() dispose.mock.resetCalls() - nested1 = await root.loader.create({ + nested1 = await loader.create({ name: 'foo', }, alpha) - nested2 = await root.loader.create({ + nested2 = await loader.create({ name: 'foo', isolate: { bar: 'beta', }, }, alpha) - nested3 = await root.loader.create({ + nested3 = await loader.create({ name: 'foo', isolate: { bar: true, @@ -276,14 +288,14 @@ describe('service isolation: realm', async () => { await new Promise((resolve) => setTimeout(resolve, 0)) expect(foo.mock.calls).to.have.length(2) expect(dispose.mock.calls).to.have.length(0) - expect(root.loader.entries[nested1]!.fork).to.be.ok - expect(root.loader.entries[nested1]!.fork!.ctx.get('bar')!.value).to.equal('alpha') - expect(root.loader.entries[nested1]!.fork!.status).to.equal(ScopeStatus.ACTIVE) - expect(root.loader.entries[nested2]!.fork).to.be.ok - expect(root.loader.entries[nested2]!.fork!.ctx.get('bar')!.value).to.equal('beta') - expect(root.loader.entries[nested2]!.fork!.status).to.equal(ScopeStatus.ACTIVE) - expect(root.loader.entries[nested3]!.fork).to.be.ok - expect(root.loader.entries[nested3]!.fork!.ctx.get('bar')).to.be.undefined - expect(root.loader.entries[nested3]!.fork!.status).to.equal(ScopeStatus.PENDING) + expect(loader.entries[nested1]!.fork).to.be.ok + expect(loader.entries[nested1]!.fork!.ctx.get('bar')!.value).to.equal('alpha') + expect(loader.entries[nested1]!.fork!.status).to.equal(ScopeStatus.ACTIVE) + expect(loader.entries[nested2]!.fork).to.be.ok + expect(loader.entries[nested2]!.fork!.ctx.get('bar')!.value).to.equal('beta') + expect(loader.entries[nested2]!.fork!.status).to.equal(ScopeStatus.ACTIVE) + expect(loader.entries[nested3]!.fork).to.be.ok + expect(loader.entries[nested3]!.fork!.ctx.get('bar')).to.be.undefined + expect(loader.entries[nested3]!.fork!.status).to.equal(ScopeStatus.PENDING) }) }) diff --git a/packages/loader/tests/utils.ts b/packages/loader/tests/utils.ts index 15aeb31..dac669a 100644 --- a/packages/loader/tests/utils.ts +++ b/packages/loader/tests/utils.ts @@ -15,6 +15,7 @@ declare module '../src/shared' { export default class MockLoader extends Loader { public modules: Dict = Object.create(null) + public config: Entry.Options[] = [] constructor(ctx: Context) { super(ctx, { name: 'cordis' })