Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure worker close gracefully with the same behaviours in both Terminal and Desktop apps #459

Merged
merged 38 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
9ffe1a8
close worker pipe on renderer pipe.end()
rafapaezbas Nov 21, 2024
b6641b4
final null push to receive end event in renderer worker pipe
rafapaezbas Nov 21, 2024
fb25f3e
improve-worker-close
maidh91 Nov 22, 2024
71084bc
lint
maidh91 Nov 22, 2024
8c44d27
close once
maidh91 Nov 22, 2024
0c512a6
comment
maidh91 Nov 22, 2024
5040e10
remove end
maidh91 Nov 22, 2024
d13822e
fix lint
maidh91 Nov 22, 2024
4d9d04b
on('error
maidh91 Nov 22, 2024
6e7bb41
revert
maidh91 Nov 22, 2024
0d5970c
revert
maidh91 Nov 22, 2024
d73bad2
gui
maidh91 Nov 22, 2024
b6e419f
on
maidh91 Nov 22, 2024
75d2696
on
maidh91 Nov 22, 2024
5c0a66b
up
maidh91 Nov 22, 2024
d245790
workerPipeEnd
maidh91 Nov 22, 2024
449d006
end
maidh91 Nov 22, 2024
3aaa8ee
u
maidh91 Nov 22, 2024
cf38e7c
Warning
maidh91 Nov 22, 2024
871fa13
move
maidh91 Nov 22, 2024
70e0769
pipe end
maidh91 Nov 22, 2024
253ecea
add cmt
maidh91 Nov 22, 2024
00a0ec6
remove close destroy
maidh91 Nov 22, 2024
5247da5
fx ENOTCONN
maidh91 Nov 22, 2024
ecfe16b
silent all errors
maidh91 Nov 22, 2024
4b70041
clean
maidh91 Nov 22, 2024
8ac6cd2
override emit
maidh91 Nov 25, 2024
630831c
add tests
maidh91 Nov 25, 2024
791cadf
timeout
maidh91 Nov 25, 2024
dab25b8
test terminal app ok
maidh91 Nov 25, 2024
c2600d4
terminal test ok
maidh91 Nov 25, 2024
cc0df15
test desktop
maidh91 Nov 25, 2024
ca500fb
ok
maidh91 Nov 25, 2024
0cbb43e
all tests
maidh91 Nov 25, 2024
a376e60
name
maidh91 Nov 25, 2024
b3dbcb5
test
maidh91 Nov 25, 2024
d127520
desktop tests ok
maidh91 Nov 25, 2024
875105e
clean
maidh91 Nov 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions gui/gui.js
Original file line number Diff line number Diff line change
Expand Up @@ -1498,15 +1498,21 @@ class PearGUI extends ReadyResource {
evt.reply('workerPipeClose')
})
pipe.on('data', (data) => { evt.reply('workerPipeData', data) })
pipe.on('end', () => { evt.reply('workerPipeData', null) })
pipe.on('error', (err) => { evt.reply('pipeError', err.stack) })
pipe.on('end', () => { evt.reply('workerPipeEnd') })
pipe.on('error', (err) => { evt.reply('workerPipeError', err.stack) })
})

electron.ipcMain.on('workerPipeId', (evt) => {
evt.returnValue = this.pipes.nextId()
return evt.returnValue
})

electron.ipcMain.on('workerPipeEnd', (evt, id) => {
const pipe = this.pipes.from(id)
if (!pipe) return
pipe.end()
})

electron.ipcMain.on('workerPipeClose', (evt, id) => {
const pipe = this.pipes.from(id)
if (!pipe) return
Expand Down
11 changes: 8 additions & 3 deletions gui/preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,19 +295,24 @@ class IPC {
return stream
}

workerRun (link) {
workerRun (link, args) {
const id = electron.ipcRenderer.sendSync('workerPipeId')
electron.ipcRenderer.send('workerRun', link)
electron.ipcRenderer.send('workerRun', link, args)
const stream = new streamx.Duplex({
write (data, cb) {
electron.ipcRenderer.send('workerPipeWrite', id, data)
cb()
},
final (cb) {
electron.ipcRenderer.send('workerPipeEnd', id)
cb()
}
})
electron.ipcRenderer.on('workerPipeError', (e, stack) => {
stream.emit('error', new Error('Worker PipeError (from electron-main): ' + stack))
})
electron.ipcRenderer.on('workerClose', () => { stream.destroy() })
electron.ipcRenderer.on('workerPipeClose', () => { stream.destroy() })
electron.ipcRenderer.on('workerPipeEnd', () => { stream.end() })
stream.once('close', () => {
electron.ipcRenderer.send('workerPipeClose', id)
})
Expand Down
10 changes: 7 additions & 3 deletions lib/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,13 @@ class Worker {
this.#unref()
})
const pipe = sp.stdio[3]
pipe.on('end', () => {
if (pipe.ended === false) pipe.end()
})
pipe.pid = sp.pid
const pipeEmit = pipe.emit
pipe.emit = function (event, ...args) {
if (event === 'error' && args[0]?.code === 'ENOTCONN') return false
return pipeEmit.apply(this, [event, ...args])
}
pipe.on('end', () => pipe.end())
return pipe
}

Expand Down
59 changes: 59 additions & 0 deletions test/03-worker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ const helloWorld = path.join(Helper.localDir, 'test', 'fixtures', 'hello-world')
const printArgs = path.join(Helper.localDir, 'test', 'fixtures', 'print-args')
const workerRunner = path.join(Helper.localDir, 'test', 'fixtures', 'worker-runner')

const workerParent = path.join(Helper.localDir, 'test', 'fixtures', 'worker-parent')
const workerChild = path.join(Helper.localDir, 'test', 'fixtures', 'worker-child')
const workerEndFromChild = path.join(Helper.localDir, 'test', 'fixtures', 'worker-end-from-child')
const workerDestroyFromChild = path.join(Helper.localDir, 'test', 'fixtures', 'worker-destroy-from-child')
const workerEndFromParent = path.join(Helper.localDir, 'test', 'fixtures', 'worker-end-from-parent')
const workerDestroyFromParent = path.join(Helper.localDir, 'test', 'fixtures', 'worker-destroy-from-parent')

const workerParentDesktop = path.join(Helper.localDir, 'test', 'fixtures', 'worker-parent-desktop')
const workerEndFromParentDesktop = path.join(Helper.localDir, 'test', 'fixtures', 'worker-end-from-parent-desktop')
const workerDestroyFromParentDesktop = path.join(Helper.localDir, 'test', 'fixtures', 'worker-destroy-from-parent-desktop')

test('worker pipe', async function ({ is, plan, teardown }) {
plan(1)
const helper = new Helper()
Expand Down Expand Up @@ -99,3 +110,51 @@ test('worker should run as a link in a terminal app', async function ({ is, plan

await Helper.untilClose(pipe)
})

//
// test worker exit gracefully for terminal app
//

test('[terminal] worker exit when child calls pipe.end()', async function () {
const { pipe } = await Helper.run({ link: workerParent, args: [workerEndFromChild] })
await Helper.untilWorkerExit(pipe)
})

test('[terminal] worker exit when child calls pipe.destroy()', async function () {
const { pipe } = await Helper.run({ link: workerParent, args: [workerDestroyFromChild] })
await Helper.untilWorkerExit(pipe)
})

test('[terminal] worker exit when parent calls pipe.end()', async function () {
const { pipe } = await Helper.run({ link: workerEndFromParent, args: [workerChild] })
await Helper.untilWorkerExit(pipe)
})

test('[terminal] worker exit when parent calls pipe.destroy()', async function () {
const { pipe } = await Helper.run({ link: workerDestroyFromParent, args: [workerChild] })
await Helper.untilWorkerExit(pipe)
})

//
// test worker exit gracefully for desktop app
//

test('[desktop] worker exit when child calls pipe.end()', async function () {
const { pipe } = await Helper.run({ link: workerParentDesktop, args: [workerEndFromChild] })
await Helper.untilWorkerExit(pipe)
})

test('[desktop] worker exit when child calls pipe.destroy()', async function () {
const { pipe } = await Helper.run({ link: workerParentDesktop, args: [workerDestroyFromChild] })
await Helper.untilWorkerExit(pipe)
})

test('[desktop] worker exit when parent calls pipe.end()', async function () {
const { pipe } = await Helper.run({ link: workerEndFromParentDesktop, args: [workerChild] })
await Helper.untilWorkerExit(pipe)
})

test('[desktop] worker exit when parent calls pipe.destroy()', async function () {
const { pipe } = await Helper.run({ link: workerDestroyFromParentDesktop, args: [workerChild] })
await Helper.untilWorkerExit(pipe)
})
2 changes: 2 additions & 0 deletions test/fixtures/worker-child/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const pipe = Pear.worker.pipe()
pipe.resume()
6 changes: 6 additions & 0 deletions test/fixtures/worker-child/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-child",
"main": "index.js",
"type": "module",
"pear": {}
}
4 changes: 4 additions & 0 deletions test/fixtures/worker-destroy-from-child/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const pipe = Pear.worker.pipe()
pipe.resume()
await new Promise((resolve) => setTimeout(resolve, 1000))
pipe.destroy()
6 changes: 6 additions & 0 deletions test/fixtures/worker-destroy-from-child/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-destroy-from-child",
"main": "index.js",
"type": "module",
"pear": {}
}
23 changes: 23 additions & 0 deletions test/fixtures/worker-destroy-from-parent-desktop/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const link = Pear.config.args[Pear.config.args.length - 1]
const pipe = Pear.worker.run(link)
pipe.resume()
await new Promise((resolve) => setTimeout(resolve, 1000))
pipe.destroy()
await untilExit(pipe)
Pear.Window.self.close()

async function untilExit (pipe, timeout = 5000) {
const start = Date.now()
while (isRunning(pipe)) {
if (Date.now() - start > timeout) throw new Error('timed out')
await new Promise((resolve) => setTimeout(resolve, 100))
}
}

function isRunning (pipe) {
try {
return process.kill(pipe.pid, 0)
} catch (err) {
return err.code === 'EPERM'
}
}
2 changes: 2 additions & 0 deletions test/fixtures/worker-destroy-from-parent-desktop/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<pear-ctrl></pear-ctrl>
<script src="./app.js" type="module"></script>
6 changes: 6 additions & 0 deletions test/fixtures/worker-destroy-from-parent-desktop/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-destroy-from-parent-desktop",
"main": "index.html",
"type": "module",
"pear": {}
}
22 changes: 22 additions & 0 deletions test/fixtures/worker-destroy-from-parent/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const link = Bare.argv[Bare.argv.length - 1]
const pipe = Pear.worker.run(link)
pipe.resume()
await new Promise((resolve) => setTimeout(resolve, 1000))
pipe.destroy()
await untilExit(pipe)

async function untilExit (pipe, timeout = 5000) {
const start = Date.now()
while (isRunning(pipe)) {
if (Date.now() - start > timeout) throw new Error('timed out')
await new Promise((resolve) => setTimeout(resolve, 100))
}
}

function isRunning (pipe) {
try {
return process.kill(pipe.pid, 0)
} catch (err) {
return err.code === 'EPERM'
}
}
6 changes: 6 additions & 0 deletions test/fixtures/worker-destroy-from-parent/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-destroy-from-parent",
"main": "index.js",
"type": "module",
"pear": {}
}
4 changes: 4 additions & 0 deletions test/fixtures/worker-end-from-child/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const pipe = Pear.worker.pipe()
pipe.resume()
await new Promise((resolve) => setTimeout(resolve, 1000))
pipe.end()
6 changes: 6 additions & 0 deletions test/fixtures/worker-end-from-child/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-end-from-child",
"main": "index.js",
"type": "module",
"pear": {}
}
23 changes: 23 additions & 0 deletions test/fixtures/worker-end-from-parent-desktop/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const link = Pear.config.args[Pear.config.args.length - 1]
const pipe = Pear.worker.run(link)
pipe.resume()
await new Promise((resolve) => setTimeout(resolve, 1000))
pipe.end()
await untilExit(pipe)
Pear.Window.self.close()

async function untilExit (pipe, timeout = 5000) {
const start = Date.now()
while (isRunning(pipe)) {
if (Date.now() - start > timeout) throw new Error('timed out')
await new Promise((resolve) => setTimeout(resolve, 100))
}
}

function isRunning (pipe) {
try {
return process.kill(pipe.pid, 0)
} catch (err) {
return err.code === 'EPERM'
}
}
2 changes: 2 additions & 0 deletions test/fixtures/worker-end-from-parent-desktop/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<pear-ctrl></pear-ctrl>
<script src="./app.js" type="module"></script>
6 changes: 6 additions & 0 deletions test/fixtures/worker-end-from-parent-desktop/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-end-from-parent-desktop",
"main": "index.html",
"type": "module",
"pear": {}
}
22 changes: 22 additions & 0 deletions test/fixtures/worker-end-from-parent/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const link = Bare.argv[Bare.argv.length - 1]
const pipe = Pear.worker.run(link)
pipe.resume()
await new Promise((resolve) => setTimeout(resolve, 1000))
pipe.end()
await untilExit(pipe)

async function untilExit (pipe, timeout = 5000) {
const start = Date.now()
while (isRunning(pipe)) {
if (Date.now() - start > timeout) throw new Error('timed out')
await new Promise((resolve) => setTimeout(resolve, 100))
}
}

function isRunning (pipe) {
try {
return process.kill(pipe.pid, 0)
} catch (err) {
return err.code === 'EPERM'
}
}
6 changes: 6 additions & 0 deletions test/fixtures/worker-end-from-parent/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-end-from-parent",
"main": "index.js",
"type": "module",
"pear": {}
}
21 changes: 21 additions & 0 deletions test/fixtures/worker-parent-desktop/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const link = Pear.config.args[Pear.config.args.length - 1]
const pipe = Pear.worker.run(link)
pipe.resume()
await untilExit(pipe)
Pear.Window.self.close()

async function untilExit (pipe, timeout = 5000) {
const start = Date.now()
while (isRunning(pipe)) {
if (Date.now() - start > timeout) throw new Error('timed out')
await new Promise((resolve) => setTimeout(resolve, 100))
}
}

function isRunning (pipe) {
try {
return process.kill(pipe.pid, 0)
} catch (err) {
return err.code === 'EPERM'
}
}
2 changes: 2 additions & 0 deletions test/fixtures/worker-parent-desktop/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<pear-ctrl></pear-ctrl>
<script src="./app.js" type="module"></script>
6 changes: 6 additions & 0 deletions test/fixtures/worker-parent-desktop/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-parent-desktop",
"main": "index.html",
"type": "module",
"pear": {}
}
20 changes: 20 additions & 0 deletions test/fixtures/worker-parent/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const link = Bare.argv[Bare.argv.length - 1]
const pipe = Pear.worker.run(link)
pipe.resume()
await untilExit(pipe)

async function untilExit (pipe, timeout = 5000) {
const start = Date.now()
while (isRunning(pipe)) {
if (Date.now() - start > timeout) throw new Error('timed out')
await new Promise((resolve) => setTimeout(resolve, 100))
}
}

function isRunning (pipe) {
try {
return process.kill(pipe.pid, 0)
} catch (err) {
return err.code === 'EPERM'
}
}
6 changes: 6 additions & 0 deletions test/fixtures/worker-parent/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "worker-parent",
"main": "index.js",
"type": "module",
"pear": {}
}
16 changes: 16 additions & 0 deletions test/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,22 @@ class Helper extends IPC.Client {
return res
}

static async isRunning (pipe) {
try {
return process.kill(pipe.pid, 0)
} catch (err) {
return err.code === 'EPERM'
}
}

static async untilWorkerExit (pipe, timeout = 5000) {
const start = Date.now()
while (await this.isRunning(pipe)) {
if (Date.now() - start > timeout) throw new Error('timed out')
await new Promise((resolve) => setTimeout(resolve, 100))
}
}

static async pick (stream, ptn = {}, by = 'tag') {
if (Array.isArray(ptn)) return this.#untils(stream, ptn, by)
for await (const output of stream) {
Expand Down