Segmentation fault when converting vector of custom type using to_v8

[alichay]

When I use v8pp::to_v8 on a vector containing a custom type (that has been registered as a v8pp::class_) I get a segmentation fault.

There's not much I can say about it, so I'll just provide some "simple" (nothing with V8 is simple, though) sample code that should demonstrate where the issue is. Here's the pastebin upload of it.

Thanks.

[pmed]

Hi @zangent,

Thanks for reporting, your sample code is pretty clear. The cause of this issue is v8pp::to_v8() returns an empty V8 handle when converting an object of custom type in the vector.

You are trying to convert unwrapped C++ objects created at line 82:

    /* Create a vector of MyObject */
    std::vector<MyObject> vec = {
        MyObject("test.cpp", "int main(){}"),
        MyObject("test.hpp", "#pragma once")
    };

but v8pp does know nothing about these objects, and v8pp::to_v8() returns empty V8 handle for such unwrapped objects.

There are 2 ways to wrap a C++ object properly in v8pp:

1. To create a JS object with v8 binding class: v8pp::class_<MyObject>::create(isolate, "test.cpp", "int main(){}")
2. To specialize the converter template v8pp::convert<MyObject> for C++ structures with data members only.

There is also a similar sample in the recent issue #39.

I'm not sure how to prevent similar further issues, probably only by adding an assert in the v8pp::to_v8() function that the V8 handle is not empty, and throwing an exception from v8pp::convert<T>::to_v8() when there is no wrapped C++ object.

[alichay]

I could be wrong here, but I figured that that was the point of doing this:

	/* Set up my MyObject class */
	v8pp::class_<MyObject> v8_MyObject(isolate);
	v8_MyObject
		.ctor<std::string, std::string>()
		.set("name", &MyObject::name)
		.set("contents", &MyObject::contents);

Do I also have to specialize v8pp::convert to get this to work? If so, that's fine, but it looked like I wouldn't need to after a glance at the source code.

[pmed]

No, you don't have to. You either can use v8pp::class_ to describe a C++ class bindings, such as data members, functions, base classes, or you can specialize v8pp::convert template for a C++ struct and create V8 objects with only data properties.

I think it's simpler to go the 2nd way for MyObject use case:

#include <iostream>
#include <vector>
#include <string>
#include <memory>

#include "v8pp/context.hpp"
#include "v8pp/module.hpp"
#include "v8pp/class.hpp"
#include "v8pp/object.hpp"

#include <libplatform/libplatform.h>

void console_log(v8::FunctionCallbackInfo<v8::Value> const& args)
{
	v8::HandleScope handle_scope(args.GetIsolate());

	for (int i = 0; i < args.Length(); ++i)
	{
		if (i > 0) std::cout << ' ';
		v8::String::Utf8Value const str(args[i]);
		std::cout << *str;
	}
	std::cout << std::endl;
}

struct MyObject {
	MyObject(std::string n, std::string c) : name(n), contents(c) {}
	std::string name;
	std::string contents;
};

// specialize v8pp convert for MyObject struct
template<>
struct v8pp::convert<MyObject>
{
	static bool is_valid(v8::Isolate* isolate, v8::Local<v8::Value> const& value)
	{
		return !value.IsEmpty() && value->IsObject();
	}

	static MyObject from_v8(v8::Isolate* isolate, v8::Local<v8::Value> const& value)
	{
		if (!is_valid(isolate, value)) throw std::runtime_error("expected Object");

		auto object = value.As<v8::Object>();
		std::string name, contents;
		if (get_option(isolate, object, "name", name)
			&& get_option(isolate, object, "contents", contents))
		{
			return MyObject(name, contents);
		}
		throw std::invalid_argument("expected {name, contents} object");
	}

	static v8::Local<v8::Object> to_v8(v8::Isolate* isolate, MyObject const& value)
	{
		auto object = v8::Object::New(isolate);
		set_option(isolate, object, "name", value.name);
		set_option(isolate, object, "contents", value.contents);
		return object;
	}
};

int main(int argc, const char * argv[])
{
	/* Initialize V8. */
	v8::V8::InitializeICU();
	v8::V8::InitializeExternalStartupData(argv[0]);
	std::unique_ptr<v8::Platform> platform(v8::platform::CreateDefaultPlatform());
	v8::V8::InitializePlatform(platform.get());
	v8::V8::Initialize();

	try
	{
		/* Set up v8pp */
		v8pp::context context;
		v8::Isolate* isolate = context.isolate();

		 /* Open handle scope and obtain global object */
		v8::HandleScope scope(isolate);
		auto global = isolate->GetCurrentContext()->Global();

		 /* Set up `console.log` */
		v8pp::module console(isolate);
		console.set("log", &console_log);
		v8pp::set_option(isolate, global, "console", console.new_instance());

		/* Create a vector of MyObject */
		std::vector<MyObject> vec = {
			MyObject("test.cpp", "int main(){}"),
			MyObject("test.hpp", "#pragma once")
		};

		/* Convert that vector to a V8 array, v8pp::convert<MyObject> is used instead of v8pp::class_<MyObject> */
		auto js_array = v8pp::to_v8(isolate, vec);

		 /* Convert JS array back to vector of MyObject */
		auto const vec2 = v8pp::from_v8<std::vector<MyObject>>(isolate, js_array);
		//assert(vec2 == vec);

		 /* Attempt to pass the V8 array to the script as a global variable */
		v8pp::set_option(isolate, global, "test_values", js_array);

		/* Run some simple code that uses the vector */
		context.run_script("for (let i = 0; i < test_values.length; i++)"
			"console.log(test_values[i].name)", "<sample_code>");
	}
	catch (std::exception & ex)
	{
		std::cerr << ex.what() << std::endl;
		return -1;
	}

	v8::V8::Dispose();
	v8::V8::ShutdownPlatform();
}

[alichay]

Ah, okay. I knew in the back of my mind that while templates were powerful, they weren't magic. Thanks for sorting this out!

Edit: Also, thanks for making V8 development bearable; this library is going to keep me from having a stress heart attack!

[pmed]

Thanks for using the library, I appreciate such a feedback from real users :)

[alichay]

One more question, since I couldn't see anything about this myself.
In this code:

static v8::Local<v8::Object> to_v8(v8::Isolate* isolate, file_source_pair const& value) {

	auto obj = v8::Object::New(isolate);
	set_option(isolate, obj, "name", value.name);
	set_option(isolate, obj, "contents", value.contents);

is there any way to set a getter/setter here instead of a straight value? I know that in v8pp::class_ you can use a v8pp::property, is there any similar way here?

Thanks for your tremendous help, by the way.

[pmed]

Well, as I understand you can use v8::Object::SetAccessor() for some V8 object with C++ callback functions to get and set a property values of that object. But this is V8 API, there are no helper functions in v8pp.