Strange assert, unmoved heapptr was selected for finalization

[harold-b]

Were there any major changes to garbage collection?

Take this with a grain of salt since usually the bugs I present is some weird thing I messed up in my side, but this was a strange one.

In my app I have certain indices in the stash reserved for some object pools. The pools get allocated at the beginning as arrays, and they are left there. I simply grab the heapptr and add/remove objects to that array. I haven't had any issues with it ever, suddenly today I got an assert triggered when I pushed the stashed pool's heapptr:

/* One particular problem case is where an object has been
         * queued for finalization but the finalizer hasn't been
         * executed.
         */
        duk_heaphdr *curr;
        for (curr = thr->heap->finalize_list;
             curr != NULL;
             curr = DUK_HEAPHDR_GET_NEXT(thr->heap, curr)) {
            DUK_ASSERT(curr != (duk_heaphdr *) ptr);
        }

Again, I might have corrupted something somewhere, who knows, just looking for some info in case there were some changes that might have caused this? This portion of my code (with the pool) hasn't been touched in a very long time...

The heapptr which is an array inside an array inside the heap stash is only ever grabbed from the stash index once at the beginning, the pushed into the stack only 3 places in the code. The stash is not messed with again other than in initializing those arrays at the beginning.

If it helps, the objects that are stored in that pool (the heapptr that caused the assert) are revived when being finalized. That is, the pool is full of preallocated Ecmascirpt objects that I grab from the pool return them when I need one, then nullify that object's index. When the object get's finalized I keep it alive by storing it back in the pool.

[svaarala]

There hasn't been a major change to GC but e.g. string table changes affected some parts of the GC.

Do you have a clean good/bad commit that could be used for bisecting?

[svaarala]

And also what was the assert in question? :)

[svaarala]

Ah, I think it's this:

        DUK_ASSERT(curr != (duk_heaphdr *) ptr);

[harold-b]

Yup, that's the assert.

I'm afraid I don't have any commits that changed anything. I haven't touched any part of that code in quite a while (many months), or anything regarding the stash for that matter. I never had encountered any issues with that part either, which is why I found it so strange.

I can't reproduce anything at all either, was seemingly random :(

On my part of the code, this was all that was executed:

    ASSERT( duk_is_object( cx, -1 ) );

    duk_push_heapptr( cx, _poolHeapPtr );   // [... obj pool]
    duk_dup( cx, -2 );                      // [... obj pool obj]
    duk_put_prop_index( cx, -2, idx );      // [... obj pool]
    duk_pop_2( cx );                        // [...]

This _poolHeapPtr is only obtained from an array in the heap stash at the beginning, and never re-assigned again. It's only used duk_push_heapptr in 3 places and that's it.

[harold-b]

There's not much info here to reproduce anything, I thought I'd ask in case there were some changes that might ring a bell with you, in case such an issue had appeared before or something. That not being the case, feel free to close the issue to not pollute your feed.

I'll report any findings if it does happen again.

[harold-b]

In case the stack trace is of any consequence (doubtful) here it is:

ucrtbased.dll!___threadid�()    Unknown
    ucrtbased.dll!___acrt_report_runtime_error�()   Unknown
    ucrtbased.dll!_abort�() Unknown
    Application.exe!duk_default_fatal_handler(void * udata, const char * msg) Line 10537    C
    Application.exe!duk_push_heapptr(duk_hthread * ctx, void * ptr) Line 20929  C
    Application.exe!Musashi::Scripting::SmallObjPool::ReleaseIndex(int idx) Line 148    C++
>   Application.exe!Musashi::Scripting::VectorSharedBindings<CoreMath::T_Vector2<float>,0>::Finalizer(duk_hthread * cx) Line 649    C++
    Application.exe!Musashi::Scripting::Vector2Bindings<CoreMath::T_Vector2<float>,0>::fin(duk_hthread * cx) Line 915   C++
    Application.exe!duk__handle_call_inner(duk_hthread * thr, int num_stack_args, unsigned int call_flags, int idx_func) Line 59505 C
    Application.exe!duk_handle_call_unprotected(duk_hthread * thr, int num_stack_args, unsigned int call_flags) Line 59038  C
    Application.exe!duk_call(duk_hthread * ctx, int nargs) Line 13283   C
    Application.exe!duk__finalize_helper(duk_hthread * ctx, void * udata) Line 49768    C
    Application.exe!duk__handle_safe_call_inner(duk_hthread * thr, int(*)(duk_hthread *, void *) func, void * udata, int idx_retbase, int num_stack_rets, unsigned int entry_valstack_bottom_index, unsigned int entry_callstack_top, unsigned int entry_catchstack_top) Line 59997 C
    Application.exe!duk_handle_safe_call(duk_hthread * thr, int(*)(duk_hthread *, void *) func, void * udata, int num_stack_args, int num_stack_rets) Line 59828    C
    Application.exe!duk_safe_call(duk_hthread * ctx, int(*)(duk_hthread *, void *) func, void * udata, int nargs, int nrets) Line 13446 C
    Application.exe!duk_hobject_run_finalizer(duk_hthread * thr, duk_hobject * obj) Line 49820  C
    Application.exe!duk__run_object_finalizers(duk_heap * heap, unsigned int flags) Line 45570  C
    Application.exe!duk_heap_mark_and_sweep(duk_heap * heap, unsigned int flags) Line 46046 C
    Application.exe!duk__run_voluntary_gc(duk_heap * heap) Line 46122   C
    Application.exe!duk_heap_mem_alloc(duk_heap * heap, unsigned int size) Line 46146   C
    Application.exe!duk_hbuffer_alloc(duk_heap * heap, unsigned int size, unsigned int flags, void * * out_bufdata) Line 43284  C
    Application.exe!duk_push_buffer_raw(duk_hthread * ctx, unsigned int size, unsigned int flags) Line 20877    C
    Application.exe!duk_push_fixed_buffer_nozero(duk_hthread * ctx, unsigned int len) Line 20891    C
    Application.exe!duk__concat_and_join_helper(duk_hthread * ctx, int count_in, int is_join) Line 21762    C
    Application.exe!duk_concat(duk_hthread * ctx, int count) Line 21807 C
    Application.exe!duk__vm_arith_add(duk_hthread * thr, duk_double_union * tv_x, duk_double_union * tv_y, unsigned short idx_z) Line 68727 C
    Application.exe!duk__js_execute_bytecode_inner(duk_hthread * entry_thread, unsigned int entry_callstack_top) Line 71674 C
    Application.exe!duk_js_execute_bytecode(duk_hthread * exec_thr) Line 70899  C
    Application.exe!duk__handle_call_inner(duk_hthread * thr, int num_stack_args, unsigned int call_flags, int idx_func) Line 59453 C
    Application.exe!duk_handle_call_protected(duk_hthread * thr, int num_stack_args, unsigned int call_flags) Line 58936    C
    Application.exe!duk_pcall_method(duk_hthread * ctx, int nargs) Line 13382   C
    Application.exe!Musashi::ScriptGameSys::Draw() Line 144 C++
    Application.exe!Musashi::DevGameSystem::Draw() Line 232 C++
    Application.exe!Musashi::_App::Draw() Line 470  C++
    Application.exe!Musashi::_App::Update(double dt) Line 421   C++
    Application.exe!Musashi::_App::ProcessMsg(Musashi::AppMsgType type, Musashi::AppMessageParam param) Line 202    C++
    Application.exe!Musashi::Win32MsgLoop() Line 255    C++
    Application.exe!Musashi::App::Run(Musashi::AppStartInfo & info, int argc, const char16_t * * argv) Line 170 C++
    Application.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 83   C++
    Application.exe!invoke_main() Line 99   C++
    Application.exe!__scrt_common_main_seh() Line 253   C++
    Application.exe!__scrt_common_main() Line 296   C++
    Application.exe!WinMainCRTStartup() Line 17 C++
    kernel32.dll!@BaseThreadInitThunk@12�() Unknown
    ntdll.dll!___RtlUserThreadStart@8�()    Unknown
    ntdll.dll!__RtlUserThreadStart@8�() Unknown

[svaarala]

@harold-b This case seems similar to the one we debugged a while ago - I'm not sure if the case is otherwise similar though.

The assert is related to checking for an API violation where the pointer given to duk_push_heapptr() has not been 100% reachable from duk_get_heapptr() to duk_push_heapptr(). If that isn't the case, the outcome is not guaranteed (anything else is not supported).

Last time when we debugged a similar issue there was something that caused the object to be pushed back before it was finalized. In other words, the argument given to duk_push_heapptr() had become unreachable, hadn't been finalized yet (which can take time in some cases), but had been pushed back on the assumption that because the finalizer hadn't been called, the pointer must still be live (which is not always the case because finalizer execution is not immediate).

The reason this causes troublie is the internal book-keeping which assumes that all objects on the finalize_list are unreachable and unreferenced until the finalizer executes. If duk_push_heapptr() is used for the pointer before it is finalized, both conditions are violated and some odd behavior could ensue.

[svaarala]

I'm afraid I don't have any commits that changed anything.

I meant a good/bad Duktape commit which would allow finding which change triggered the issue (if this is indeed caused by some change in Duktape behavior).

[harold-b]

I meant a good/bad Duktape commit which would allow finding which change triggered the issue (if this is indeed caused by some change in Duktape behavior).

Well, I've been running from the duk_opt_xxx branch for over a week now and didn't see any issues whatsoever with it. Before that i was running off v1.5.0. No issues either. It just happened out of nowhere.

Regarding reachability, the heapptr in question points to an array inside an array inside the heap stash, these are allocated at initialization and never touched again. These objects are never passed into any Ecmascript code either. They exist in the stash for the lifetime of the app, so I don't think it's related to that either. They are just plain arrays added with duk_push_array.

[harold-b]

...Having set this. I still wouldn't put it past me to have some weird thing messed up somewhere... we both know my reputation regarding these issues... :sweat_smile:

[svaarala]

Ok, but I'm not sure I follow - you said the arrays contain object references that are rescued in the finalizer? So this assert would most likely be related to such a pointer, that gets pushed before the finalizer has actually executed.

[harold-b]

Well, the assert occurred when I pushed the heapptr for the array object that lives in the stash:

   ASSERT( duk_is_object( cx, -1 ) );

    duk_push_heapptr( cx, _poolHeapPtr );   // [... obj pool]  <-- HERE
    duk_dup( cx, -2 );                      // [... obj pool obj] <-- Never got here
    duk_put_prop_index( cx, -2, idx );      // [... obj pool]
    duk_pop_2( cx );                        // [...]

obj in this context is the object that is being finalized. It's left in the stack, and then 'heapDestructing' parameter is popped:

/// This is the finalizer code:

// Stack: [obj heapDestruct:bool]
duk_pop( cx );  // [obj]

// Handle recycling the object
ScriptContext* sc     = Scripting::GetStashedSystem( cx );
SmallObjPool*  pool   = sc->GetPoolByIndex( (StashPoolIdx)PoolIdx );

duk_get_prop_string( cx, 0, MUSX_SO_POOL_PNAME_IDX );   // [obj index]
    ASSERT( duk_is_number( cx, -1 ) );
int idx = (int)duk_require_int( cx, -1 );

duk_pop( cx );  // [obj]

// Release native object back to the pool &
// recycle the jsobject back to it's pool.
// jsobject must be on top of the stack
pool->ReleaseIndex( idx );  // [] <-- Here it goes to the snipped above where the assert occurred

return 0;

Perhaps you can spot something I did wrong here by chance?

[svaarala]

After re-reading the description I can answer to the above myself :) The assert is triggered by pushing the heapptr of an array which is guaranteed to remain reachable at all times.

The only logical explanations would be heap corruption caused by e.g. pushing an object that has already become unreachable (but not yet finalized), or some bug in Duktape.

For the latter, since the object is in the finalize_list, the bug wouldn't probably be in refcounting (because the finalizer would execute without going through finalize_list) but in mark-and-sweep. The most logical bug would be a failure to mark the object for some reason even though it's reachable.

[harold-b]

The only logical explanations would be heap corruption caused by e.g. pushing an object that has already become unreachable (but not yet finalized), or some bug in Duktape.

Probably the former, hehe... But I suspect it must be some elusive, hard to spot, corruption on my side somewhere.

[svaarala]

@harold-b If there's a bug in your application, it wouldn't necessarily be in the finalizer - but instead in the code before finalization which decides that it's safe to use duk_push_heapptr().

In your case you're using duk_get_heapptr(), then using duk_push_heapptr() somewhere (these call sites would be worth checking), and then allowing the reference to become unreachable and wait for the finalizer to rescue it. (The bug is not necessarily related to the array heapptr reference, it may be collateral damage from handling other objects.)

The tricky part is that when you duk_push_heapptr(), you have no direct way of knowing the pointer hasn't become unreachable already because it may take some time before the finalizer gets executed. That's why the API contract is that you shouldn't ever duk_push_heapptr() unless the reference is guaranteed to be reachable 100% of the time between duk_get_heapptr() and duk_push_heapptr().

To be clear, your approach should work provided that you can guarantee that duk_push_heapptr() is only used when the object has been guaranteed to be reachable the whole time, or in the finalizer (never in between). (Technically the finalizer call is a violation but it works because it's basically equivalent to calling duk_get_heapptr() again and getting the same result back.)

[harold-b]

(The bug is not necessarily related to the array heapptr reference, it may be collateral damage from handling other objects.)

That's what i meant by some elusive corruption on my side =/.

Those pool objects never get moved again in the stash, in fact, the stash isn't touched after initialization. So I'm betting whatever caused is is such a corruption elsewhere. The unfortunate thing is that it just happened once, seemingly at random. The data returned from the object that was being finalized (it's index) was correct as well (I inspected it when it asserted), so we'll see if it ever pops up again.

I'll close this now, thanks for having a look. I'll let you know if anything comes up again regarding this.

[svaarala]

Thanks @harold-b. I wouldn't rule out a Duktape bug either. If there was an outright marking bug it would usually manifest pretty quickly. But it's still quite possible there is a bit more elusive bug somewhere (couple of these have been found and fixed over the years).

If you get something that is repeatable, it'll be much easier to pin down the issue.

[fatcerberus]

@harold-b If it helps you manage heap pointers any better, feel free to use these ref/unref functions I came up with. Just replace your duk_get_heapptr() calls with duk_ref_heapptr() and you get a pointer that's guaranteed to remain reachable until you explicitly unref it (note: it's refcounted too). This way you don't have to manage the stash manually.

duk_ref_heapptr(): https://github.com/fatcerberus/minisphere/blob/master/src/engine/utility.c#L157-L200

duk_unref_heapptr(): https://github.com/fatcerberus/minisphere/blob/master/src/engine/utility.c#L235-L271

[harold-b]

Thanks Bruce, I already have a function pair that does that very thing :)

[harold-b]

@svaarala Something happened that might help me track where my corruptions lies. It just so happens that a typescript file was not emitted in my output, ( the main App file ) and the javascript output was missing it as the main entry point so when running my script from duktape it immediately throws because 'App' is undefined. However, some small bits of code are run before the App object is created, some decorators that register Entity types. Before the call is returning, it asserts in duk_handle_safe_call :

duk__handle_safe_call_inner(thr,
                                    func,
                                    udata,
                                    idx_retbase,
                                    num_stack_rets,
                                    entry_valstack_bottom_index,
                                    entry_callstack_top,
                                    entry_catchstack_top);

        /* Longjmp state is kept clean in success path */
        DUK_ASSERT(thr->heap->lj.type == DUK_LJ_TYPE_UNKNOWN); // <-- Right here

So it seems I'm corrupting something pretty early in the game. My app is able to hot-reload the scripts and I do it all the time without issue, even when I've had transpilation issues when a file did not get emmited and it simply throws on reload. So that means it might have been a recent change I made somewhere that is corrupting, perhaps it won't be too hard too find.

So I just wanted to ask you, from that assert, do you know what kind of actions I might have done with the API that lead to this type of heap corruption? (Other than a buffer overrun or something like that completely unrelated to duktape). Would me pushing an invalid heap_ptr cause this? Or one that has been collected or something? Any info that could point me to a good place to checking in my code would help :)

[harold-b]

Actually, I've isolated my offending JS code, so I'm on the right path. Just need to track now where I mess up on the native side.

[svaarala]

There might be a number of causes for something like this - but one possible sequence is that Duktape's heap tracking linked lists get out of sync (due to e.g. an invalid duk_push_heapptr()) and that then, much later, causes an object to be deemed unreachable when it is logically reachable.

There was a bug like this in Duktape refcounting (IIRC) in Duktape 1.x, found by the Frida guys, which caused weird behavior for an object that was rescued multiple times. So it could also be a Duktape bug, but probably something similar to this one rather than an overt bug like forgetting to mark something (though that's possible too, just seems unlikely).

It'd really help if you can pin down a repeatable case :)

[harold-b]

Yup I'm on the case now! I've got it pinpointed, I doubt it's an issue with duktape. But I finally isolated it completely and can reproduce. I'm tracing the native path now to see what i broke.

[harold-b]

Is there any debug macro that asserts on pushing an invalid heapptr with duk_push_heapptr()?

[svaarala]

The only assert in place is that finalize_list check that got triggered for the (known reachable) array you were pushing. Other than that Duktape doesn't detect that a pushed pointer is in fact unreachable already; it would need to run mark-and-sweep to make that determination.

Anyway, if you can now repeat the problem, you would now be in place to check if the problem appeared in a certain Duktape version, IOW if there's a good/bad pair for Duktape commits/versions. If so, tracking down the commit might be possible, if this is a Duktape bug.

[harold-b]

OK, I'm currently stripping code paths til I get the most isolated possible. I'll run with a clean v2.0.0 build next to see if it happens there.

[fatcerberus]

Such an assert is not really feasible, as the same memory (and pointer) can be reused after it's freed. This is especially true with pool allocators but can also happen with standard malloc(). So it's not really possible to assert for without creating a lot of false positives.

[svaarala]

That's true - but it would be possible to detect an unreachable heapptr which wasn't yet in the finalize_list and not yet freed. Current code detects the finalize_list case. But then there are cases where the pointer is already freed, and all bets are off like @fatcerberus said (may be reused, may be reused as a different type of object, etc).

[harold-b]

Right, what I mean was more for the case when a heapptr is deem invalid because it's unreachable or is currently freed. If it has been re-used that's another story, but as far as pushing it is concerned at that point, it's a valid pointer, it may just not be the pointer you are expecting.

[svaarala]

Hmm, so to clarify the cases:

• The pointer is reachable: OK.
• The pointer is unreachable but not yet detected as such by Duktape: this would be detectable by an assert but would involve doing a mark-and-sweep-like operation. I'm not sure if anything would go wrong in this case actually, though with refcounts enabled this would only happen for an object participating in a reference loop, or when debugger was paused (which blocks refzero operations).
• The pointer is unreachable, has a finalizer, queued to finalize_list but not yet finalized: there's an assert in place for that already. The assert walks the finalize_list which can be heavy but typically isn't because the finalize_list is typically empty or short.
• The pointer has been freed, but not yet reused: this would be detectable by running through all the heap objects, and by seeing no such pointer exists in the heap. This would work even if the memory area was allocated again for some other purpose than a heap object, as the pointer wouldn't then be in any heap tracking list.
• The pointer has been freed, but allocated again for a heap object, i.e. reused: this can't be detected from a plain pointer alone, and like you said it would be memory safe but the resulting object would be different than you expected.

[svaarala]

Out of those cases, commenting on the likely problems here:

• Unreachable but not detected: I don't think anything would actually go wrong here.
• Unreachable, queued to finalize_list: the current assert should cover this.
• Freed but not yet reused: this would cause problems because the object would go into the heap but it's link pointers would be bogus which could cause very, very weird issues: when the object is freed it may cause link pointers of other objects to be incorrectly updated which causes very weird issues down the line.
• Freed and reused for a heap object: this would be memory safe but cause an incorrect object to be pushed.
• Freed and reused for a non-heap-object: this would cause memory unsafe behavior; the link pointers would be invalid and when written would corrupt the non-heap allocation. And vice versa, writing to the non-heap-object would overwrite heap pointers that Duktape relies on.

[svaarala]

@harold-b One crude way of maybe detecting if you're accidentally holding a pointer after it has been freed would be to wrap your current free function that Duktape uses, and whenever a pointer is freed, walk through your application's stored heapptrs to see if the freed pointer (which may be something else than a heap object but that shouldn't matter) matches any of them.

[harold-b]

@svaarala So I just finished testing with duktape v2.0.0 and i got the same issue. Then I tested with v1.5.0, no issue... Hmm..

[harold-b]

@harold-b One crude way of maybe detecting if you're accidentally holding a pointer after it has been freed would be to wrap your current free function that Duktape uses, and whenever a pointer is freed, walk through your application's stored heapptrs to see if the freed pointer (which may be something else than a heap object but that shouldn't matter) matches any of them.

OK let me check, I got it to the point thatthe only time duk_push_heapptr() is used is in 3 locations which are in the object binder class which pushes the prototype object, the constructor and a stash location in which they are saved in an array.

[harold-b]

I isolated the issue occurring only with a duk_error call. If I didn't call that in a particular function, no assert happened. With duktape 1.5.0, no such error occurs and the throw happens normally.

I'm gonna switch back to v2.0.0 to further investigate. (Will start by checking said heapptrs...)

[svaarala]

@harold-b So if this is a Duktape issue, you should be able to bisect the offending commit from the commit sequence between, say, v1.5.0 and v2.0.0 tags. They are both in the main commit chain (not in a maintenance branch) so while there are many commits, it's a linear sequence of merges. The annoying part is that the configuration model changes in between.

[harold-b]

@svaarala Let me investigate a little bit more with v2.0.0... I want to be more certain I'm not screwing something up.

[svaarala]

Sure, let me know if there's something I could try to reproduce - and if you are able to narrow the good/bad Duktape commits by bisecting that would might provide enough of a hint too. I'll check back tomorrow :)

[harold-b]

Thanks for all the help :) I'll continue updating with any findings.

[harold-b]

Got it. I found the culprit and here's a reproducible case.

This was tested with:

 *  Git commit 4180966c47d6d87106008dd4338de8d507c8072b (v2.0.0).
 *  Git branch HEAD.

// Configuration:
-DDUK_USE_FASTINT                   \
-DDUK_USE_ASSERTIONS

Te assert occurs in duk_js_call.c at line 1953. thr->heap->lj.type == DUK_LJ_TYPE_THROW in this case.

This happens in both Release and Debug builds. Tested on Win7, VisualStudio 2015 with both 32 and 64bit, getting the same result.

To reproduce:

#include "duktape.h"

int x = 0;

static duk_ret_t Foo_ctor( duk_context* cx )
{
    // [0]
    return duk_error( cx, DUK_ERR_TYPE_ERROR, "No Foo for you!" );
}

static duk_ret_t Foo_fin( duk_context* cx )
{
    // stack: [ targetObject:Object, forciblyFreed:bool ]

    // Avoid being optimized out in release build.
    x = *((int*)cx);

    return 0;
}

int main( int argc, const char* argv[] )
{
    const char script[] = "function() { new Foo(); }";

    duk_context* cx = duk_create_heap_default();

    duk_push_global_object( cx );               // [glob]

    // Add constructor and prototype object
    duk_push_c_function( cx, Foo_ctor, 0 );     // [glob ctor]
    duk_push_object( cx );                      // [glob ctor proto]

    // Set finalizer
    duk_push_c_function( cx, Foo_fin, 2 );      // [glob ctor proto fin]
    duk_set_finalizer( cx, -2 );                // [glob ctor proto]

    // Set constructor.prototype
    duk_put_prop_string( cx, -2, "prototype" ); // [glob ctor]

    // Put Foo in the global object
    duk_put_prop_string( cx, -2, "Foo" );       // [glob]

    duk_pop( cx );  // []

    // Compile
    duk_push_string( cx, "main.js" );   // [name]
    duk_pcompile_lstring_filename( cx, DUK_COMPILE_FUNCTION, script, sizeof(script)-1 );  // [func]

    // Run
    duk_pcall( cx, 0 ); // [result]
    duk_pop( cx );      // []

    duk_destroy_heap( cx );

    return x;
}

This happens after the finalizer call, which happens after the constructor call. If there is no finalizer bound, or if we don't throw from the constructor, no issue occurs.

[fatcerberus]

Based on that repro case, it asserts when you throw from the constructor of an object with a finalizer?

[harold-b]

Also, under VS2015 I'm getting a C4703 error for act in duk_handle_ecma_call_setup(). I don't get this when compiling my App, not sure if I disabled that warning level or something, but I got it for the test case. It seems the compiler's static analyzer is not good enough to determine that it is in fact set before you get to the second if (use_tailcall)

[harold-b]

@fatcerberus The constructor is called, and then finalizer is also called before the assertion is caught.

[fatcerberus]

MSVC default warning level is 3, so that's probably why you didn't see it before.

[harold-b]

@fatcerberus Hmm... Just checked the solution and the warning level was set to 3 and Treat Warnings As Errors was also false... weird... :confused:

[harold-b]

Know of any other option that might be allowing it?

[harold-b]

Ah, it was "SDL Checks"

[fatcerberus]

Here's a repro case based on your C code that works in pure JS (which @svaarala should find nice :)

function Foo() { throw new Error("a pig ate it"); }
Duktape.fin(Foo.prototype, function(o) {});
new Foo();

[harold-b]

Here's a repro case based on your C code that works in pure JS (which @svaarala should find nice :)

Perfect, thought that might be the case as well.

[fatcerberus]

The bug seems to manifest whenever a constructor throws whose automatic this has a finalizer (which in practice requires the finalizer to be set on the prototype). Things seem to work fine in a build without assertions enabled, but that doesn't mean something unsafe isn't happening behind the scenes.