Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes for #72: Callbacks are being GC'd #73

Closed
wants to merge 2 commits into from

2 participants

@jahewson

Two fixes associated with #72

  • tests now build on Windows (MSVC disagrees with GCC about vararg pointers)
  • print a message before segfaulting due to GC'd callbacks
@TooTallNate
Owner

@jahewson Thanks for these, but I actually fixed things a little differently as you saw in 502b68f and 2ed61e4.

The only thing now is that when invoking one of the "sprintf" tests, I get an error like this!

Freaking Windows... In any case, this pull is no longer needed. Thanks anyways though!

@TooTallNate
Owner

Strangely, df52f86 fixes that error message.

@jahewson

No worries, happy to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 16, 2012
  1. @jahewson

    build varadic test on Windows

    jahewson authored
  2. @jahewson
This page is out of date. Refresh to see the latest.
Showing with 16 additions and 2 deletions.
  1. +9 −1 src/callback_info.cc
  2. +7 −1 test/ffi_tests.cc
View
10 src/callback_info.cc
@@ -39,7 +39,15 @@ void CallbackInfo::DispatchToV8(callback_info *info, void *retval, void **parame
argv[1] = WrapPointer((char *)parameters, sizeof(char *) * info->argc);
TryCatch try_catch;
-
+
+ if (*info->function == NULL) {
+ // callback function has been gc'd, the call to info->function->Call
+ // will result in a segfault which cannot be avoided. It's the
+ // responsibility of the JavaScript callee to keep a reference to
+ // its callback around until it knows the foreign code is done with it.
+ fprintf(stderr, "FATAL ERROR - ffi callback has been garbage-collected\n");
+ }
+
// invoke the registered callback function
info->function->Call(Context::GetCurrent()->Global(), 2, argv);
View
8 test/ffi_tests.cc
@@ -162,8 +162,14 @@ void Initialize(Handle<Object> target) {
target->Set(String::NewSymbol("abs"), WrapPointer((char *)absPtr));
// snprintf pointer; used in the varadic tests
+#ifdef _WIN32
+ // Windows has multiple `_snprintf_s` signatures, so we need to manually disambiguate
+ int (*snprintfPtr)(char*, size_t, size_t, const char*)(snprintf);
+ target->Set(String::NewSymbol("snprintf"), WrapPointer((char *)snprintfPtr));
+#else
target->Set(String::NewSymbol("snprintf"), WrapPointer((char *)snprintf));
-
+#endif
+
// hard-coded `strtoul` binding, for the benchmarks
NODE_SET_METHOD(target, "strtoul", Strtoul);
Something went wrong with that request. Please try again.