Closing FileHandle in addon's Finalize method

[th3r0b0t]

I have a simple addon accepting a FileHandle object in its constructor:

filelock::filelock(const Napi::CallbackInfo& info) : Napi::ObjectWrap<filelock>(info)
{
    Napi::Env env = info.Env();
    jsFileHandle1 = Napi::Persistent( info[0].As<Napi::Object>() );

    if (env.IsExceptionPending())
    {
        Napi::Error e = env.GetAndClearPendingException();
        std::cerr << "ERROR2: "<< e.Message() << std::endl;
    }
    return;
}

And the Finalize method:

void filelock::Finalize(Napi::Env env)
{
    std::cerr << "inside Finalize" << std::endl;
    Napi::HandleScope scope(env);
    
    Napi::Value closeFD = this->jsFileHandle1.Value().Get("close");
    if( closeFD.IsFunction() ) { std::cerr << "type ok!" << std::endl; }
    try {
            closeFD.As<Napi::Function>().Call(this->jsFileHandle1.Value(), {});
            if (env.IsExceptionPending())
           {
              Napi::Error e = env.GetAndClearPendingException();
              std::cerr << "ERROR: "<< e.Message() << std::endl;
            }
          }
    catch (const Napi::Error& e)
    {
            e.ThrowAsJavaScriptException();
    }
    return;
}

Running this code after I saved Persistent ref. to FileHandle, works ok with VScode JS debugger, but causing core dump when ran in normal terminal!

---

Secondly: When C++ exceptions aren't enabled (cmake-js default) how should I catch errors? not try-catch nor IsExceptionPending works in the above code and I get terminate called after throwing an instance of 'Napi::Error'.

Third: Is calling close safe in a basic Finalizer? As it worked just the same as above! What APIs considered safe and not accessing JS heap (I mean is there an easy way to tell if API is accessing heap)?

My humblest and greatest regards.

[mhdawson]

We discussed in the node-api team meeting today.

• For the question about how to check when exceptions aren not enabled, this documentation provides some info - https://github.com/nodejs/node-addon-api/blob/main/doc/error_handling.md#handling-errors-with-maybe-type-and-c-exceptions-disabled
• For the question about the crash, we need some more information to be able to know what to look at. Can you answer these questions:
  • What Node.js version are you using?
  • What version of node-addon-api are you using?
  • Have you enabled experimental
  • Do you have a stack trace and can you share it with us?

[th3r0b0t]

@mhdawson I'm very thankful for you attention and effort;

I tried to follow exactly as the document you mentioned, says; At Handling Errors Without C++ Exceptions it says:

Instead, it raises pending JavaScript exceptions and returns an empty Napi::Value. The calling code should check env.IsExceptionPending() before attempting to use a returned value
...
If the pending exception is not cleared, it will be thrown when the native code returns to JavaScript."

You can see I used GetAndClearPendingException as doc's example code and with or without it, I can't throw the error!
You can see the Finalize in the linked repo at cpp-src/filelock.cpp.

---

As for infos you requested:

• Node v22.2.0
• node-addon-api latest (package-lock.json says 8.3.1)
• No; everything on its default
• As for stack trace, I once got some stack trace (before using asyncWorker and promise) but can't anymore; Here's the whole code: https://github.com/th3r0b0t/filelock/tree/dev

[th3r0b0t]

There's a second problem which I open another issue for it if I should, but since it is just another stupid question I didn't want to bother you more than I do.
I defined OnCalledAsFunction like this:
class definition:

class filelock : public Napi::ObjectWrap<filelock>
{
  public:
//...
    static Napi::Value OnCalledAsFunction(const Napi::CallbackInfo& callbackInfo);
//...
};

Method definition:

Napi::Value filelock::OnCalledAsFunction(const Napi::CallbackInfo& callbackInfo)
{
    std::cerr << "OnCalledAsFunction" << std::endl;
    Napi::TypeError::New(callbackInfo.Env(), "filelock constructor cannot be invoked without 'new'").ThrowAsJavaScriptException();
    return callbackInfo.Env().Undefined();
}

Now if I call the ctor without new I get the following error and program stuck:

OnCalledAsFunction
(node:38063) Warning: Closing file descriptor 29 on garbage collection
(Use `node --trace-warnings ...` to show where the warning was created)
(node:38063) [DEP0137] DeprecationWarning: Closing a FileHandle object on garbage collection is deprecated. Please close FileHandle objects explicitly using FileHandle.prototype.close(). In the future, an error will be thrown if a file descriptor is closed during garbage collection.

This is the exact same behavior I get as when I don't define OnCalledAsFunction explicitly except the log line being printed!
Here's the code that calls the ctor:

export async function create(metadata, body)
{
    let metaFileFD;
    try { metaFileFD = await open(requestedFile, constants.O_CREAT | constants.O_RDWR, 0o600); }
    catch(err){throw err;}
    let lock = await filelock(metaFileFD).acquireWriteLock();
}

[legendecas]

I think it is hard for people to understand what the issue is when conflating multiple issues together. Would you mind clarifying what the issue is and what you expect from node-addon-api? Thank you very much!

[th3r0b0t]

I think it is hard for people to understand what the issue is when conflating multiple issues together. Would you mind clarifying what the issue is and what you expect from node-addon-api? Thank you very much!

@legendecas
You are right; I apologize.
Let's focus on the first problem, calling close method on a objectReference to a FileHandle.

[mhdawson]

From our discussion in the team meeting today we are still thinking its likely a problem in the addon itself. Could you run with valgrind to see if it helps point out where memory might be being used after it has been freed?

Some info on how to do that is in https://github.com/nodejs/node/blob/main/doc/contributing/investigating-native-memory-leaks.md

[th3r0b0t]

From our discussion in the team meeting today we are still thinking its likely a problem in the addon itself. Could you run with valgrind to see if it helps point out where memory might be being used after it has been freed?

Some info on how to do that is in https://github.com/nodejs/node/blob/main/doc/contributing/investigating-native-memory-leaks.md

@mhdawson
Thank you a lot for your time.
Sure I'll do that.