Skip to content

Commit 6857a77

Browse files
dlespiaury
authored andcommitted
worker: Fix ownership of the ArrayBuffer backing storage in the go->js path (ry#29)
We witnessed double frees on buffers passed to the JS recv() function: jkcfg/jk#71 jkcfg/jk#91 With a bit of intrumentation (https://github.com/dlespiau/debug-allocations), we traced this down to both, the go code and the JS garbage collection freeing the buffer. And indeed we call C.free and create the AB telling the JS garbage collector it owns the storage with ArrayBufferCreationMode::kInternalized. One of the two has to go. Given C.CBytes() allocates the buffer on the C stack using malloc and the v8 allocator is the libc malloc/free we can safely pass this buffer to v8. This will allow recv() users to not have to care about copying content of that AB before recv() returns. I traced the malloc/free of that buffer and confirmed they are properly freed (in this case a 340 bytes buffer with address 0x7fc914000a40): malloc 340 0x7fc914000a40 ./log-malloc.so(+0xb4e)[0x7fc9219dab4e] ./log-malloc.so(malloc+0xa3)[0x7fc9219dac6b] jk(_cgo_9d48446f0bab_Cfunc__Cmalloc+0x14)[0x9ccd54] jk[0x6dcc00] [...] free 0x7fc914000a40 ./log-malloc.so(+0xb4e)[0x7fc9219dab4e] ./log-malloc.so(free+0x8f)[0x7fc9219dae1f] jk(_ZN2v88internal20ArrayBufferCollector15FreeAllocationsEv+0x8e)[0xf9807e] jk(_ZN2v88internal20ArrayBufferCollector11FreeingTask11RunInternalEv+0x146)[0xf98326] jk(_ZN2v88platform12WorkerThread3RunEv+0x36)[0x9d38f6] jk[0xe0b260] /lib/x86_64-linux-gnu/libpthread.so.0(+0x76ba)[0x7fc9217c46ba] /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7fc920c5941d]
1 parent e3fa6c4 commit 6857a77

File tree

1 file changed

+4
-1
lines changed

1 file changed

+4
-1
lines changed

worker.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,11 @@ func (w *Worker) LoadModule(scriptName string, code string, resolve ModuleResolv
244244
// Same as Send but for []byte. $recv callback will get an ArrayBuffer.
245245
func (w *Worker) SendBytes(msg []byte) error {
246246
msg_p := C.CBytes(msg)
247-
defer C.free(msg_p)
248247

248+
// C.CBytes allocates memory on the C heap that is used as the backing
249+
// storage for the ArrayBuffer given to javascript. v8 will free the buffer
250+
// as part of the ArrayBuffer garbage collection as we create the AB with
251+
// ArrayBufferCreationMode::kInternalized in worker_send_bytes.
249252
r := C.worker_send_bytes(w.worker.cWorker, msg_p, C.size_t(len(msg)))
250253
if r != 0 {
251254
errStr := C.GoString(C.worker_last_exception(w.worker.cWorker))

0 commit comments

Comments
 (0)