Possible memory leak in Array.prototype.map?

[dolsem]

What version of Bun is running?

1.0.29+a146856d1

What platform is your computer?

Linux 5.15.133.1-microsoft-standard-WSL2 x86_64 unknown

What steps can reproduce the bug?

I found a pretty bizarre issue when playing around with ffi. Here's a simple implementation of execlp in Bun using ffi:

import { dlopen, ptr, Pointer } from 'bun:ffi';

function stringToPtr(value: string) {
  const buf = new Uint8Array(Buffer.byteLength(value) + 1);
  new TextEncoder().encodeInto(value, buf);
  return ptr(buf);
}

export function execlp(...args: [command: string, arg0: string, ...args: string[]]): void {
  const suffix = process.platform === 'darwin' ? 'dylib' : 'so.6';
  const { execlp } = dlopen(`libc.${suffix}`, {
    execlp: {
      args: Array(args.length + 1).fill('ptr'),
      returns: 'i32',
    },
  }).symbols;

  const argPtrs = args.map(stringToPtr); // <- Problematic line!

  argPtrs.push(0 as Pointer);

  if (execlp(...argPtrs) < 0) {
    throw new Error('execlp failed');
  }
}

// adding another call to `.map()` with an arbitrary array here causes the issue
execlp('/usr/bin/bash', 'bash', '-c', 'echo Hello World');

This works perfectly fine UNLESS you make a call to .map() anywhere in your code that returns object values. For example, if I add one of the following before the call to execlp, execlp will fail:

const a = [{}].map(x => x);
const b = [1].map(x => null);

However, these work fine, execlp succeeds:

const c = [{}].map(x => 1);
const d = [{}];

If I get rid of the call to .map() in the execlp implementation, execlp will work fine, e.g. replace that line with the following:

const argPtrs = [] as Pointer[];
for (const arg of args) {
  argPtrs.push(stringToPtr(arg));
}

What is the expected behavior?

.map() shouldn't cause execlp to fail.

What do you see instead?

error: execlp failed

Additional information

No response

[Jarred-Sumner]

function stringToPtr(value: string) {
  const buf = new Uint8Array(Buffer.byteLength(value) + 1);
  new TextEncoder().encodeInto(value, buf);
  return ptr(buf);
}

This is a use-after-free. buf gets freed sometime after this function is called. ptr no longer points to a valid memory address

[dolsem]

@Jarred-Sumner oh I thought the pointer has a strong reference to the value. In any case, as long as stringToPtr() is called without using .map(), or there are no other calls to .map() anywhere in the program like I described, execlp() seems to work fine with the current implementation of stringToPtr(), I guess there is not enough time to garbage collect the value. Fixing the implementation doesn't get rid of the issue, e.g. the following still causes execlp to fail:

import { dlopen, ptr, Pointer } from 'bun:ffi';

function stringToPtr(value: string, store: Uint8Array[]) {
  const buf = new Uint8Array(Buffer.byteLength(value) + 1);
  store.push(buf);
  new TextEncoder().encodeInto(value, buf);
  return ptr(buf);
}

export function execlp(...args: [command: string, arg0: string, ...args: string[]]): void {
  const suffix = process.platform === 'darwin' ? 'dylib' : 'so.6';
  const { execlp } = dlopen(`libc.${suffix}`, {
    execlp: {
      args: Array(args.length + 1).fill('ptr'),
      returns: 'i32',
    },
  }).symbols;

  const bufs = [] as Uint8Array[];
  const argPtrs = args.map((arg) => stringToPtr(arg, bufs)); // <- call to map
  argPtrs.push(0 as Pointer);

  if (execlp(...argPtrs) < 0) {
    throw new Error('execlp failed');
  }
}

const a = [{}].map(x => x);
execlp('/usr/bin/bash', 'bash', '-c', 'echo Hello World')