Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

AssemblyScript: GC finalizer for Response #50

Open
radu-matei opened this issue Apr 2, 2021 · 8 comments
Open

AssemblyScript: GC finalizer for Response #50

radu-matei opened this issue Apr 2, 2021 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@radu-matei
Copy link
Member

AssemblyScript/assemblyscript#1256 introduced a new GC hook that runs before managed objects before being collected.
This is helpful for Response objects, whose handles have to be manually closed right now, and in theory, we could remove this requirement by implementing the finalizer.

The issue is that Response objects don't seem to be collected - i.e. the console log is never executed here.

@global function __finalize(ptr: usize): void {
    // @ts-ignore
    if (__instanceof(ptr, idof<Response>())) {
        Console.log("running finalizer with ptr: " + ptr.toString())
        // change type to `Response` and call `close`
    }

// @ts-ignore
__collect();

Manually keeping track of Response objects and the collected pointers, it's clear that they are not collected with the current implementation, which could be an indicator that we are actually leaking memory.

Another question is related to manually calling __collect - without manually calling it, the finalizer hook is not always executed. Are there any best practices around manually calling it or not?

cc @jedisct1

@radu-matei radu-matei added the help wanted Extra attention is needed label Apr 2, 2021
@jedisct1
Copy link
Contributor

jedisct1 commented Apr 2, 2021

Maybe @MaxGraey would be the one to ask here :)

@MaxGraey
Copy link
Contributor

MaxGraey commented Apr 2, 2021

сс @dcodeIO

@dcodeIO
Copy link

dcodeIO commented Apr 2, 2021

Finalization should happen eventually, once the GC kicks in. With the incremental runtime, when this happens depends, though. With default settings, the first threshold is at about half the memory used for example, so depending on the program's allocation pattern, the GC may kick in early, or even never. Subsequent GCs happen when used memory doubles again, relative to remaining memory used after the previous collection. Probably not good enough to rely upon for closing handles.

@radu-matei
Copy link
Member Author

Thanks a lot for your reply, @dcodeIO!

I guess we have to keep the requirement that consumers have to manually close the handle for now, until we have a better solution, such as a destructor.

@jedisct1
Copy link
Contributor

jedisct1 commented Apr 6, 2021

That issue is common to virtual alll WASI APIs, that heavily depend on handles.

In Zig, instead of finalizers, we have defer and errdefer statements that can automatically free a resource before a function returns, or on error.

Maybe this is something that could also be implemented in AssemblyScript?

@MaxGraey
Copy link
Contributor

MaxGraey commented Apr 6, 2021

defer is just early declaration for releasing resource (explicit RAII) and this the same as manually close such resource before return but with some more continent way. So

fn foo() -> bool {
  let res = io.open(path)
  defer(res)
  // some code
  if (fail) return false // will implicitly call io.close(res) before return. Injected in compile time
  // some other code
  return true // will implicitly call io.close(res) before return
}

and it's the same as:

fn foo() -> bool {
  let res = io.open(path)
  // some code
  if (fail) {
    io.close(res)
    return false
  }
  // some other code
  io.close(res)
  return true
}

and this works only inside user space

@MaxGraey
Copy link
Contributor

MaxGraey commented Apr 6, 2021

JavaScript / Typescript usually return callback function which contine code for finalization:

export function foo(): () => bool {
  const desc = fs.open(path);
  // some code
  return () => {
     fs.close(desc);
     return true;
  };
}

while AS don't support closures. It may be temporary workaround:

let tmpDesc: FileDesc | null = null; // global var

export function foo(): () => bool {
  tmpDesc = fs.open(path); // store to global desc
  // some code
  return () => {
     fs.close(tmpDesc); // reat from global desc
     return true;
  };
}

@MaxGraey
Copy link
Contributor

MaxGraey commented Apr 6, 2021

If you required many descriptors simultaneously it may be something like this:

let descMap: Map<i32, FileDesc> = new Map; // it could be stack / queue
let gId = 0;

export function foo(): (id: i32) => bool {
  descMap.set(gId++, fs.open(path)); // store to global desc
  // some code
  return (id: i32) => {
     if (descMap.has(id)) {
       let desc = descMap.get(id);
       fs.close(desc); // reat from global desc
       descMap.delete(id);
       return true;
     }
     return false;
  };
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants