GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — ffi: fix crash in refCallback and unrefCallback · nodejs/node@428e9bc

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

doc/api
- ffi.md
src
- node_ffi.cc
test/ffi
- test-ffi-weakref-calls.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎doc/api/ffi.md‎`

Copy file name to clipboardExpand all lines: doc/api/ffi.md

+7Lines changed: 7 additions & 0 deletions

Display the source diff
Display the rich diff

Original file line number	Diff line number	Diff line change
`@@ -460,6 +460,10 @@ memory.`
`460`	`460`
`461`	`461`	`Keeps the callback strongly referenced by JavaScript.`
`462`	`462`
	`463`	+Throws `ERR_INVALID_ARG_VALUE` if the callback function has already been
	`464`	+garbage collected after a previous `library.unrefCallback(pointer)` call, since
	`465`	`+a collected function cannot be referenced again.`
	`466`	`+`
`463`	`467`	### `library.unrefCallback(pointer)`
`464`	`468`
`465`	`469`	* `pointer` {bigint}
`@@ -470,6 +474,9 @@ If the callback function is later garbage collected, subsequent native`
`470`	`474`	`invocations become a no-op. Non-void return values are zero-initialized before`
`471`	`475`	`returning to native code.`
`472`	`476`
	`477`	+Throws `ERR_INVALID_ARG_VALUE` if the callback function has already been
	`478`	`+garbage collected.`
	`479`	`+`
`473`	`480`	`## Calling native functions`
`474`	`481`
`475`	`482`	`Argument conversion depends on the declared FFI type.`

Collapse file

`‎src/node_ffi.cc‎`

Copy file name to clipboardExpand all lines: src/node_ffi.cc

+17Lines changed: 17 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -1127,6 +1127,15 @@ void DynamicLibrary::RefCallback(const FunctionCallbackInfo<Value>& args) {`
`1127`	`1127`	`return;`
`1128`	`1128`	`}`
`1129`	`1129`
	`1130`	`+ // The callback function may already have been collected after a previous`
	`1131`	`+ // unrefCallback() call. The persistent handle is empty in that case, and`
	`1132`	`+ // ClearWeak() on an empty handle dereferences a null slot. There is also no`
	`1133`	`+ // function left to make strong again.`
	`1134`	`+ if (existing->second->fn.IsEmpty()) {`
	`1135`	`+ THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found");`
	`1136`	`+ return;`
	`1137`	`+ }`
	`1138`	`+`
`1130`	`1139`	`existing->second->fn.ClearWeak();`
`1131`	`1140`	`}`
`1132`	`1141`
`@@ -1158,6 +1167,14 @@ void DynamicLibrary::UnrefCallback(const FunctionCallbackInfo<Value>& args) {`
`1158`	`1167`	`return;`
`1159`	`1168`	`}`
`1160`	`1169`
	`1170`	`+ // The callback function may already have been collected by a previous`
	`1171`	`+ // unrefCallback() call. The persistent handle is empty in that case, and`
	`1172`	`+ // SetWeak() on an empty handle dereferences a null slot.`
	`1173`	`+ if (existing->second->fn.IsEmpty()) {`
	`1174`	`+ THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found");`
	`1175`	`+ return;`
	`1176`	`+ }`
	`1177`	`+`
`1161`	`1178`	`existing->second->fn.SetWeak();`
`1162`	`1179`	`}`
`1163`	`1180`

Collapse file

`‎test/ffi/test-ffi-weakref-calls.js‎`

Copy file name to clipboardExpand all lines: test/ffi/test-ffi-weakref-calls.js

+31Lines changed: 31 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,37 @@ test('ffi refCallback retains callback function', async (t) => {`
`51`	`51`	`lib.unregisterCallback(pointer);`
`52`	`52`	`});`
`53`	`53`
	`54`	`+test('callback ref/unref throw after callback function is collected', async (t) => {`
	`55`	`+ const { lib } = ffi.dlopen(libraryPath, fixtureSymbols);`
	`56`	`+ t.after(() => lib.close());`
	`57`	`+`
	`58`	`+ let callback = () => 1;`
	`59`	`+ const ref = new WeakRef(callback);`
	`60`	`+ const pointer = lib.registerCallback(`
	`61`	`+ { arguments: ['i32'], return: 'i32' },`
	`62`	`+ callback,`
	`63`	`+ );`
	`64`	`+`
	`65`	`+ lib.unrefCallback(pointer);`
	`66`	`+ callback = null;`
	`67`	`+`
	`68`	`+ await gcUntil(`
	`69`	`+ 'callback ref/unref throw after callback function is collected',`
	`70`	`+ () => ref.deref() === undefined,`
	`71`	`+ );`
	`72`	`+`
	`73`	`+ t.assert.throws(() => lib.unrefCallback(pointer), {`
	`74`	`+ code: 'ERR_INVALID_ARG_VALUE',`
	`75`	`+ message: /Callback not found/,`
	`76`	`+ });`
	`77`	`+ t.assert.throws(() => lib.refCallback(pointer), {`
	`78`	`+ code: 'ERR_INVALID_ARG_VALUE',`
	`79`	`+ message: /Callback not found/,`
	`80`	`+ });`
	`81`	`+`
	`82`	`+ lib.unregisterCallback(pointer);`
	`83`	`+});`
	`84`	`+`
`54`	`85`	`test('callback ref/unref/unregister throw when library is closed', (t) => {`
`55`	`86`	`const { lib } = ffi.dlopen(libraryPath, fixtureSymbols);`
`56`	`87`	`const callback = lib.registerCallback(() => {});`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.