facebook/hermes

[HermesABI][C API] Creating PropName from string triggers assertion failure

robik opened this issue · 6 comments

Bug Description

  • [?] I have run gradle clean and confirmed this bug does not occur with JSC
  • [?] The issue is reproducible with the latest version of React Native.

Hermes git revision (if applicable): main (f2970e7)
React Native version: none
OS: MacOs
Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64): arm64-v8a

Steps To Reproduce

Run below code repro

#include <stdio.h>
#include <hermes_abi.h>
#include <hermes_vtable.h>

int main() {
    const struct HermesABIVTable *global_vt = get_hermes_abi_vtable();
    struct HermesABIRuntime *rt = global_vt->make_hermes_runtime(NULL);

    struct HermesABIStringOrError strOrError = rt->vt->create_string_from_utf8(rt, "test", 4);
    if ((strOrError.ptr_or_error & 1) == 1) {
        printf("Failed to create str");
        return 1;
    }
    struct HermesABIString* hermesStr = (struct HermesABIString*)strOrError.ptr_or_error;

    // FAILS HERE
    struct HermesABIPropNameIDOrError propNameOrError = rt->vt->create_propnameid_from_string(rt, *hermesStr);

    // Never reached
    if ((propNameOrError.ptr_or_error & 1) == 1) {
        printf("Failed to create propname");
        return 1;
    }

    struct HermesABIPropNameID* propName = (struct HermesABIPropNameID*)propNameOrError.ptr_or_error;

    rt->vt->release(rt);
    return 0;
}

The Expected Behavior

Expected: PropName is created
Current: Assertion failure

The assertion failure is related to ref counting I guess:

Assertion failed: (!isFree() && "Value not present"), function value, file hermes_vtable.cpp, line 95.

Hey @robik, it's great to see that you're experimenting with the C API. Like with JSI, the C-API requires that all references you create are freed before the runtime is destroyed. You can do this by calling the invalidate vtable method on HermesABIManagedPointer.

In your example, both propName and hermesStr need to be freed before calling release on the runtime.

Hey @neildhar, thanks for clarifying!

I skipped invalidation as it is not relevant in this case.

The app crashes at call to the create_propname_from_string function, never reaching runtime release call

Ah, I see, there seems to be another issue. The line

    struct HermesABIString* hermesStr = (struct HermesABIString*)strOrError.ptr_or_error;

Should actually be

    struct HermesABIString hermesStr{(struct HermesABIManagedPointer*)strOrError.ptr_or_error};

ptr_or_error is a HermesABIManagedPointer *, which is supposed to be a field of a HermesABIString, not a pointer to a HermesABIString. (similarly for the PropNameID)

In our defense, this C ABI wasn't meant to be used directly, but to build other APIs, JSI in particular, on top of it :-)

Do we need to improve the documentation?

Okay, thank you very much! This makes sense.

I did not use documentation, just (poorly) reverse engineered it. Thanks for the clarification!

@robik You may also find it helpful to refer to the JSI implementation in HermesABIRuntime.cpp, and to use the C++ helper functions in HermesABIHelpers.h. Apart from that, it's exciting that you're trying out the C-API, and we're keen to hear how it goes!