S
SolidJS•6d ago
webstrand

How to onCleanup an async resource from createResource?

createResource(url, async (url) => {
    var resource;
    tryOnCleanup(() => resource?.close());
    return resource = await connectToUrl(url);
});
createResource(url, async (url) => {
    var resource;
    tryOnCleanup(() => resource?.close());
    return resource = await connectToUrl(url);
});
or 
createResource(url, async (url, info) => {
    tryOnCleanup(() => info.value?.close()); // not sure if this reference gets updated?
    return await connectToUrl(url);
});
createResource(url, async (url, info) => {
    tryOnCleanup(() => info.value?.close()); // not sure if this reference gets updated?
    return await connectToUrl(url);
});
or 
const [conn] = createResource(url, async (url) => await connectToUrl(url));
createComputed(() => { const c = conn(); tryOnCleanup(() => c?.close()) })
const [conn] = createResource(url, async (url) => await connectToUrl(url));
createComputed(() => { const c = conn(); tryOnCleanup(() => c?.close()) })
or something else entirely? I need to cleanup the connection when the resource re-fetches, and when the whole subtree is disposed of. If I don't the browser leaks the resource and eventually crashes.
41 Replies
bigmistqke
bigmistqke•6d ago
i would have done 1. but I wasn't aware of this second argument in the fetcher from the docs it says
The value property tells you the previously fetched value.
so ig you should not use onCleanup then: 
createResource(url, async (url, previous) => {
    previous.value?.close()
    return await connectToUrl(url);
});
createResource(url, async (url, previous) => {
    previous.value?.close()
    return await connectToUrl(url);
});
webstrand
webstrandOP•6d ago
yeah I can confirm that option 2 definitely doesn't work it's hard following memory leaks, but option 1 results in an 8mb process, option 2 results in 16gb process
peerreynders
peerreynders•5d ago
yeah I can confirm that option 2 definitely doesn't work
… or there is something strange going on intryOnCleanup This PoC works as intended: https://playground.solidjs.com/anonymous/67705400-1213-473b-8ca9-e949723b7446 
async function allocate(arg: number) {
  await new Promise<void>((r) => setTimeout(r, 2000));
  const handle = Math.trunc(arg / 1000) % 86400;
  console.log('Allocating handle', handle);
  return handle;
}

const disposeOf = (handle: number) => console.log('Disposed of', handle);

function scheduleDisposal(handle: number) {
  // `handle` captures identity of resource
  // that needs to be disposed of - until
  // it's disposed of…
  setTimeout(disposeOf, 1000, handle);
}

function App() {
  const [arg, setArg] = createSignal(Date.now());
  const [handle] = createResource<number, number>(arg, (arg, info) => {
    if (info.value) scheduleDisposal(info.value);
    return allocate(arg);
  });

  return (
    <Show when={!handle.loading && handle()} fallback={'…loading'}>
      <button type="button" onClick={() => setArg(Date.now())}>
        {handle()}
      </button>
    </Show>
  );
}
async function allocate(arg: number) {
  await new Promise<void>((r) => setTimeout(r, 2000));
  const handle = Math.trunc(arg / 1000) % 86400;
  console.log('Allocating handle', handle);
  return handle;
}

const disposeOf = (handle: number) => console.log('Disposed of', handle);

function scheduleDisposal(handle: number) {
  // `handle` captures identity of resource
  // that needs to be disposed of - until
  // it's disposed of…
  setTimeout(disposeOf, 1000, handle);
}

function App() {
  const [arg, setArg] = createSignal(Date.now());
  const [handle] = createResource<number, number>(arg, (arg, info) => {
    if (info.value) scheduleDisposal(info.value);
    return allocate(arg);
  });

  return (
    <Show when={!handle.loading && handle()} fallback={'…loading'}>
      <button type="button" onClick={() => setArg(Date.now())}>
        {handle()}
      </button>
    </Show>
  );
}
Solid Playground
Quickly discover what the solid compiler will generate from your JSX template
webstrand
webstrandOP•5d ago
That PoC leaks memory when used with onCleanup, for example: https://playground.solidjs.com/anonymous/96d8f185-c590-4ae6-acfc-6c53fd4f9dc3 note especially that destroying the whole component fails to cleanup the last value in handle2 and otherwise retains two handles
bigmistqke
bigmistqke•5d ago
notice how peer and my example do not have the cleanup in an onCleanup it's probably still not the way for u to go, bc our examples only cleanup when the resource is re-triggered, not when the resource itself is cleaned up.
peerreynders
peerreynders•5d ago
What is motivating you to use onCleanup in that particular manner? I'm not sure it was designed to be used that way. I see disposing on rerun and on component disposal as two separate concerns. onCleanup is appropriate for the latter … https://playground.solidjs.com/anonymous/57135dd8-578d-41a8-b13a-6b8c85e66863
Solid Playground
Quickly discover what the solid compiler will generate from your JSX template
bigmistqke
bigmistqke•5d ago
I see disposing on rerun and on component disposal as two separate concerns.
mechanically these are the same tho? both is an owner that cleans up
peerreynders
peerreynders•4d ago
The first is “I'm done with this asset because I'm getting another one”. The other is “I'm done with the context that is using the asset”.
bigmistqke
bigmistqke•4d ago
i am not talking semantically but mechanically
peerreynders
peerreynders•4d ago
I'm talking when vs. how.
webstrand
webstrandOP•4d ago
I need the resource created to be destroyed when no longer necessary. And due to browser issues I can't rely on "garbage collection" to deal with it, the tab just crashes after a bit without manually cleaning up when the resource goes "out of scope" either due to the component that owns the resource being removed or the resource being refetched, I must perform the cleanup operation or the browser leaks the resource using setTimeout doesn't work because the resource will likely be used after an arbitrary period of time, the only time that I can cleanup the resource is when its owner gets disposed, or the value gets refetched
peerreynders
peerreynders•4d ago
The issue with onCleanup is that you set cleanup for the next rerun. But in your option 2 input.value is from the previous run; so your cleanup runs “one behind”. Option 1 is only OK if you synchronously dispose otherwise there could be a racing condition on var handle. That's why I don't like onCleanup in there.
webstrand
webstrandOP•4d ago
yeah. But what is the alternative to onCleanup? I do realize that if the refetch happens inbetween registering the onCleanup and the await resolving, then the resource gets lost
bigmistqke
bigmistqke•4d ago
no you are right, for option 2. you should not use onCleanup option 1. is probably the way to go because otherwise you don't cleanup when the resource cleans up
peerreynders
peerreynders•4d ago
https://playground.solidjs.com/anonymous/57135dd8-578d-41a8-b13a-6b8c85e66863 Use onCleanup on the component level to dispose of the current instance. Within the rerun dispose of the previous instance directly.
Solid Playground
Quickly discover what the solid compiler will generate from your JSX template
bigmistqke
bigmistqke•4d ago
i think this is valid too: 
createResource(url, async (url) => {
    var resource;
    onCleanup(() => resource?.close());
    return resource = await connectToUrl(url);
});
createResource(url, async (url) => {
    var resource;
    onCleanup(() => resource?.close());
    return resource = await connectToUrl(url);
});
webstrand
webstrandOP•4d ago
That's an option, it's just not scheduling the dispose after resource is no longer accessible 
  const [handle1] = createResource(arg1, async (arg) => {
    let isDead = false;
    var handle: number | undefined;
    onCleanup(() => {
      if(handle !== undefined) dispose(handle)
      isDead = true;
    });
    handle = await allocate(arg);
    if(isDead) dispose(handle);
    return handle;
  });
  const [handle1] = createResource(arg1, async (arg) => {
    let isDead = false;
    var handle: number | undefined;
    onCleanup(() => {
      if(handle !== undefined) dispose(handle)
      isDead = true;
    });
    handle = await allocate(arg);
    if(isDead) dispose(handle);
    return handle;
  });
the await allows other code to run, before the promise resolves. The resource could be re-fetched or disposed of in that inbetween space, causing the onCleanup to run before resource gets assigned
peerreynders
peerreynders•4d ago
This is what you want 
createResource(url, async (url) => {
  const resource = await connectToUrl(url);
  onCleanup(() => resource?.close());
  return resource;
});
createResource(url, async (url) => {
  const resource = await connectToUrl(url);
  onCleanup(() => resource?.close());
  return resource;
});
bigmistqke
bigmistqke•4d ago
no that won't work
webstrand
webstrandOP•4d ago
that onCleanup never runs, since its outside of Owner = context
bigmistqke
bigmistqke•4d ago
because the onCleanup would be behind the async boundary not following there ooo now i see
webstrand
webstrandOP•4d ago
yeah. I haven't actually seen that behavior in the wild, but it's probably only a matter of time
bigmistqke
bigmistqke•4d ago
ok ok now i understand the full problem ye i guess this is actually not a bad approach then ugly but it does cover the case stuff like 
const [conn] = createResource(url, async (url) => await connectToUrl(url));
createComputed(() => { const c = conn(); tryOnCleanup(() => c?.close()) })
const [conn] = createResource(url, async (url) => await connectToUrl(url));
createComputed(() => { const c = conn(); tryOnCleanup(() => c?.close()) })
you are also not sure if the promise is resolved before the resource is called again you have to do it inside the resource
peerreynders
peerreynders•4d ago
Why use var (instead of let)?
bigmistqke
bigmistqke•4d ago
i copied OP
peerreynders
peerreynders•4d ago
So there is nothing wrong with this? 
createResource(url, async (url) => {
    let resource;
    onCleanup(() => resource?.close());
    return resource = await connectToUrl(url);
});
createResource(url, async (url) => {
    let resource;
    onCleanup(() => resource?.close());
    return resource = await connectToUrl(url);
});
bigmistqke
bigmistqke•4d ago
no it should probably be this because of this
webstrand
webstrandOP•4d ago
I think that does bad things to suspense, right? need to check the status first or it'll block loading of the whole component
bigmistqke
bigmistqke•4d ago
also true
peerreynders
peerreynders•4d ago
Sounds like you want to delay disposal 
  let lastHandle: number | undefined;
  const dispose = (handle: number | undefined) => {
    if (!handle) return;
    if (handle === lastHandle) lastHandle = undefined;
    console.log('disposing', handle);
  };
  const [arg1, setArg1] = createSignal(Date.now());
  const [handle1] = createResource<number, number>(arg1, async (arg, info) => {
    const handle = await allocate(arg);
    setTimeout(dispose, 0, info?.value);
    return lastHandle = handle;
  });
  onCleanup(() => dispose(lastHandle));
  let lastHandle: number | undefined;
  const dispose = (handle: number | undefined) => {
    if (!handle) return;
    if (handle === lastHandle) lastHandle = undefined;
    console.log('disposing', handle);
  };
  const [arg1, setArg1] = createSignal(Date.now());
  const [handle1] = createResource<number, number>(arg1, async (arg, info) => {
    const handle = await allocate(arg);
    setTimeout(dispose, 0, info?.value);
    return lastHandle = handle;
  });
  onCleanup(() => dispose(lastHandle));
webstrand
webstrandOP•4d ago
oof, actually, on refetch getOwner() returns null, so no solution involving onCleanup in the body of the resource fetcher is legal odd that the dev-mode warnings aren't enabled in the playground
peerreynders
peerreynders•4d ago
webstrand
webstrandOP•4d ago
ah that's helpful, thanks
bigmistqke
bigmistqke•4d ago
that's surprising!
webstrand
webstrandOP•4d ago
https://playground.solidjs.com/anonymous/1a217951-b559-48ef-b603-8ed454c5b027 yeah, I think this is probably a bug https://github.com/solidjs/solid/issues/2428 found a better solution (aside from the refetch bug) 
  const [handle1, ctrl1] = createResource(ref, async (arg) => {
    const task = allocate(arg);
    onCleanup(() => task.then(value => console.log("cleanup", value)));
    return await task;
  });
  const [handle1, ctrl1] = createResource(ref, async (arg) => {
    const task = allocate(arg);
    onCleanup(() => task.then(value => console.log("cleanup", value)));
    return await task;
  });
peerreynders
peerreynders•4d ago
That can still clean up before the next task has finished its await.
webstrand
webstrandOP•4d ago
that's true, I need to check that createResource discards the result of the promise, if it's already refetching I assumed it discarded it, because I can't imagine otherwise it discards them, 
createComputed(() => console.log("saw handle", handle1.state === "ready" && handle1()))
createComputed(() => console.log("saw handle", handle1.state === "ready" && handle1()))
doesn't report observing any intermediate values produced by fetches that were refetched while running I'm pretty sure this is safe
peerreynders
peerreynders•4d ago
I guess it's OK if you don't use latest.
webstrand
webstrandOP•4d ago
https://playground.solidjs.com/anonymous/3cdd6c45-18b2-432f-9663-2f6e1661562b is the playground I'm working on
Solid Playground
Quickly discover what the solid compiler will generate from your JSX template
webstrand
webstrandOP•4d ago
I see what you mean about latest, I'm not sure that's fixable given the api createResource offers
peerreynders
peerreynders•4d ago
I wonder if “non-nullable” createAsync for 2.0 is going to have an impact. With 1.x createAsync reverts to undefined during rerun/refetch; my suspicion is that with 2.x it behaves more like a stale-while-revalidate, i.e. the new pending promise is still thrown to trigger suspense but the stale value is still available on the accessor.

Did you find this page helpful?