Skip to content

Commit

Permalink
fix: fix potential memleak in ctx.plugin()
Browse files Browse the repository at this point in the history
  • Loading branch information
shigma committed May 21, 2022
1 parent 605c736 commit 9f5070f
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 8 deletions.
9 changes: 3 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "cordis",
"description": "Infrastructure for Modern JavaScript Frameworks",
"version": "1.0.0",
"version": "1.1.0",
"main": "lib/index.cjs",
"module": "lib/index.ejs",
"typings": "lib/index.d.ts",
Expand Down Expand Up @@ -29,19 +29,16 @@
},
"devDependencies": {
"@types/chai": "^4.3.1",
"@types/cross-spawn": "^6.0.2",
"@types/fs-extra": "^9.0.13",
"@types/mocha": "^9.1.1",
"@types/node": "^17.0.31",
"c8": "^7.11.2",
"chai": "^4.3.6",
"cross-spawn": "^7.0.3",
"chai-shape": "^1.0.0",
"dtsc": "^1.1.0",
"esbuild": "^0.14.38",
"esbuild-register": "^3.3.2",
"fs-extra": "^10.1.0",
"globby": "^11.1.0",
"json5": "^2.2.1",
"jest-mock": "^27.5.1",
"mocha": "^9.2.2",
"rimraf": "^3.0.2",
"typescript": "^4.6.4"
Expand Down
2 changes: 1 addition & 1 deletion src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class Registry extends Map<Plugin, Plugin.State> {
context.on('service', (name) => {
if (!using.includes(name)) return
context.state.children.slice().map(plugin => this.dispose(plugin))
context.state.disposables.slice(1).map(dispose => dispose())
context.state.disposables.splice(1, Infinity).map(dispose => dispose())
callback()
})
}
Expand Down
9 changes: 8 additions & 1 deletion tests/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { App, Context } from '../src'
import { App, Context, Service } from '../src'
import { expect, use } from 'chai'
import * as jest from 'jest-mock'
import { inspect } from 'util'
Expand All @@ -18,6 +18,7 @@ const app = new App()

describe('Plugin API', () => {
it('apply functional plugin', () => {
const app = new App()
const callback = jest.fn()
const options = { foo: 'bar' }
app.plugin(callback, options)
Expand All @@ -27,6 +28,7 @@ describe('Plugin API', () => {
})

it('apply object plugin', () => {
const app = new App()
const callback = jest.fn()
const options = { bar: 'foo' }
const plugin = { apply: callback }
Expand All @@ -37,13 +39,15 @@ describe('Plugin API', () => {
})

it('apply functional plugin with false', () => {
const app = new App()
const callback = jest.fn()
app.plugin(callback, false)

expect(callback.mock.calls).to.have.length(0)
})

it('apply object plugin with true', () => {
const app = new App()
const callback = jest.fn()
const plugin = { apply: callback }
app.plugin(plugin, true)
Expand All @@ -53,12 +57,15 @@ describe('Plugin API', () => {
})

it('apply invalid plugin', () => {
const app = new App()
expect(() => app.plugin(undefined)).to.throw()
expect(() => app.plugin({} as any)).to.throw()
expect(() => app.plugin({ apply: {} } as any)).to.throw()
})

it('context inspect', () => {
const app = new App()

expect(inspect(app)).to.equal('Context <root>')

app.plugin(function foo(ctx) {
Expand Down
71 changes: 71 additions & 0 deletions tests/service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { App, Context, Service } from '../src'
import { expect } from 'chai'
import * as jest from 'jest-mock'

declare module '../src/context' {
interface Context {
foo: any
}
}

describe('Service API', () => {
it('normal service', async () => {
class Foo extends Service {
constructor(ctx: Context) {
super(ctx, 'foo')
}
}

const app = new App()
app.plugin(Foo)
expect(app.foo).to.be.undefined

await app.start()
expect(app.foo).to.be.instanceOf(Foo)

app.dispose(Foo)
expect(app.foo).to.be.undefined
})

it('immediate service', async () => {
class Foo extends Service {
constructor(ctx: Context) {
super(ctx, 'foo', true)
}
}

const app = new App()
app.plugin(Foo)
expect(app.foo).to.be.instanceOf(Foo)
})

it('dependency update', async () => {
Context.service('foo')

const callback = jest.fn()
const dispose = jest.fn(() => {})

const app = new App()
app.using(['foo'], (ctx) => {
callback(ctx.foo.bar)
ctx.on('dispose', dispose)
})

expect(callback.mock.calls).to.have.length(0)
expect(dispose.mock.calls).to.have.length(0)

app.foo = { bar: 100 }
expect(callback.mock.calls).to.have.length(1)
expect(callback.mock.calls[0][0]).to.equal(100)
expect(dispose.mock.calls).to.have.length(0)

app.foo = { bar: 200 }
expect(callback.mock.calls).to.have.length(2)
expect(callback.mock.calls[1][0]).to.equal(200)
expect(dispose.mock.calls).to.have.length(1)

app.foo = null
expect(callback.mock.calls).to.have.length(2)
expect(dispose.mock.calls).to.have.length(2)
})
})

0 comments on commit 9f5070f

Please sign in to comment.