Use WeakRef to wrap parent and children Loggers

[tegefaulkes]

Specification

Previously the each Logger kept a reference to it's parent and created children. This caused a memory leak where all derived Loggers formed a giant object group.

To fix this we removed the ability to track the children, allowing the children to be properly freed when done with. However we needed to remove the ability to reduce duplicate logger instances.

Moving forward we want to maintain the original functionality but wrap the Logger references with WeakRef() so they can still be garbage collected when needed. This means that at any time the reference can no longer exist. You can get the original reference with weakRefLogger.deref(), this can return the Logger or Undefined if it was already GC'ed.

Ultimately the children Logger will be tracked with Record<string, WeakRef<Logger>. When creating a child we check this record for the existing Logger and deref it. If it still exist then that is returned. If it was GC'ed then a new one is created and it is replaced in the record.

Additional context

• Related: Memory Leak Tracking and Remote Debugging Polykey#598
• Related: 273d054
• Related: e597b1c

Tasks

1. Reimplement child tracking but use WeakRef() to allow garbage collection.

[CMCDragonkai]

So currently logger.getChild('abc'); logger.getChild('abc') ends up getting you 2 different loggers with the same 'abc' key.

This isn't exactly correct, the original intention is that the key allows you reference a specific logger so you can re-use if necessary.

However, this caused a memory leak because there was no way to deallocate a child - which would happen if we always used a dynamic key.

To allow child loggers to be automatically garbage collected, we removed the child tracking.

So to regain this functionality, we can use the new feature called WeakRef.

The main idea is that logger.getChild('key') should create a new child logger with the name key, or get you an existing child logger with the name key, while also not preserving its existence if all other references are dropped.

ChatGPT explained that you can do this:

class Logger {
  private children: Map<string, WeakRef<LoggerChild>>;
  private registry: FinalizationRegistry<string>;

  constructor() {
    this.children = new Map();
    this.registry = new FinalizationRegistry((key: string) => {
      // Callback to remove the reference from the map
      this.children.delete(key);
    });
  }

  getChild(key: string): LoggerChild {
    let childRef = this.children.get(key);
    let childLogger = childRef?.deref();

    if (!childLogger) {
      // Create a new child logger
      childLogger = new LoggerChild(key);
      // Store a weak reference to it
      childRef = new WeakRef(childLogger);
      this.children.set(key, childRef);
      // Register the child logger for cleanup
      this.registry.register(childLogger, key);
    }

    return childLogger;
  }
}

class LoggerChild {
  constructor(private key: string) {
    // LoggerChild implementation
  }
  // Additional methods for LoggerChild
}

Notice the usage of FinalizationRegistry.

[CMCDragonkai]

So I got this working, but testing this is difficult.

The --expose-gc doesn't work with --experimental-vm-modules atm.

But I tested this with just a regular prototyping script and tsx:

import Logger from './src';

async function main() {
  const logger = new Logger('root');
  const f = () => {
    logger.getChild('child');
  };
  f();
  await new Promise(resolve => setTimeout(resolve, 0));
  globalThis.gc!();
}

void main();

And after running it I can see a deletion is occurring.

[nix-shell:~/Projects/js-logger]$ npm run tsx -- --expose-gc ./test-something.ts 

> @matrixai/[email protected] tsx
> tsx --expose-gc ./test-something.ts

THIS IS BEING DELETED

[CMCDragonkai]

So I'll add it in for the ESM version and also the CSM version. @tegefaulkes can monitor that the memory situation to ensure that this didn't bring in a regression.

[tegefaulkes]

is the FinalizationRegistry really needed here? It's not a problem to have the a GCed WeakRef remain in the child map.

[CMCDragonkai]

It's necessary for GC of the key.