Commit 105a20a9 authored by Philip Chimento's avatar Philip Chimento

fundamental: Fix broken hash table

There was a hash table (attached to the GjsContext using qdata) which
stored a mapping of void* fundamental instance pointers to JSObject
instances. This would have broken quite badly as soon as any of those
JSObjects were moved by the garbage collector.

Instead we use a JS::GCHashMap, and since it was previously attached to
the GjsContext anyway we go ahead and make it a member of
GjsContextPrivate. We put it inside a JS::WeakCache so that it will be
automatically swept when the objects are garbage collected.

The WeakCache must be destroyed before calling JS_DestroyContext() or we
will fail an assertion in debug mode.
parent 0decc66c
......@@ -33,6 +33,7 @@
#include "gi/object.h"
#include "gi/repo.h"
#include "gi/wrapperutils.h"
#include "gjs/context-private.h"
#include "gjs/jsapi-class.h"
#include "gjs/jsapi-wrapper.h"
#include "gjs/mem-private.h"
......@@ -41,63 +42,6 @@
#include <util/log.h>
#include <girepository.h>
GJS_USE
static GQuark
gjs_fundamental_table_quark (void)
{
static GQuark val = 0;
if (!val)
val = g_quark_from_static_string("gjs::fundamental-table");
return val;
}
GJS_USE
static GHashTable *
_ensure_mapping_table(GjsContext *context)
{
GHashTable *table =
(GHashTable *) g_object_get_qdata ((GObject *) context,
gjs_fundamental_table_quark());
if (G_UNLIKELY(table == NULL)) {
table = g_hash_table_new(NULL, NULL);
g_object_set_qdata_full((GObject *) context,
gjs_fundamental_table_quark(),
table,
(GDestroyNotify) g_hash_table_unref);
}
return table;
}
static void
_fundamental_add_object(void *native_object, JSObject *js_object)
{
GHashTable *table = _ensure_mapping_table(gjs_context_get_current());
g_hash_table_insert(table, native_object, js_object);
}
static void
_fundamental_remove_object(void *native_object)
{
GHashTable *table = _ensure_mapping_table(gjs_context_get_current());
g_hash_table_remove(table, native_object);
}
GJS_USE
static JSObject *
_fundamental_lookup_object(void *native_object)
{
GHashTable *table = _ensure_mapping_table(gjs_context_get_current());
return (JSObject *) g_hash_table_lookup(table, native_object);
}
/**/
FundamentalInstance::FundamentalInstance(JSContext* cx, JS::HandleObject obj)
: GIWrapperInstance(cx, obj) {
GJS_INC_COUNTER(fundamental_instance);
......@@ -110,16 +54,20 @@ FundamentalInstance::FundamentalInstance(JSContext* cx, JS::HandleObject obj)
* future if you have a pointer to @gfundamental. (Assuming @object has not been
* garbage collected in the meantime.)
*/
void FundamentalInstance::associate_js_instance(JSObject* object,
bool FundamentalInstance::associate_js_instance(JSContext* cx, JSObject* object,
void* gfundamental) {
m_ptr = gfundamental;
g_assert(_fundamental_lookup_object(gfundamental) == NULL);
_fundamental_add_object(gfundamental, object);
GjsContextPrivate* gjs = GjsContextPrivate::from_cx(cx);
if (!gjs->fundamental_table().putNew(gfundamental, object)) {
JS_ReportOutOfMemory(cx);
return false;
}
debug_lifecycle(object, "associated JSObject with fundamental");
ref();
return true;
}
/**/
......@@ -275,11 +223,10 @@ bool FundamentalInstance::constructor_impl(JSContext* cx,
GArgument ret_value;
GITypeInfo return_info;
if (!invoke_constructor(cx, object, argv, &ret_value))
if (!invoke_constructor(cx, object, argv, &ret_value) ||
!associate_js_instance(cx, object, ret_value.v_pointer))
return false;
associate_js_instance(object, ret_value.v_pointer);
GICallableInfo* constructor_info = get_prototype()->constructor_info();
g_callable_info_load_return_type(constructor_info, &return_info);
......@@ -290,7 +237,6 @@ bool FundamentalInstance::constructor_impl(JSContext* cx,
FundamentalInstance::~FundamentalInstance(void) {
if (m_ptr) {
_fundamental_remove_object(m_ptr);
unref();
m_ptr = nullptr;
}
......@@ -480,9 +426,10 @@ gjs_object_from_g_fundamental(JSContext *context,
if (gfundamental == NULL)
return NULL;
JS::RootedObject object(context, _fundamental_lookup_object(gfundamental));
if (object)
return object;
GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
auto p = gjs->fundamental_table().lookup(gfundamental);
if (p)
return p->value();
gjs_debug_marshal(GJS_DEBUG_GFUNDAMENTAL,
"Wrapping fundamental %s.%s %p with JSObject",
......@@ -496,14 +443,16 @@ gjs_object_from_g_fundamental(JSContext *context,
if (!proto)
return NULL;
object = JS_NewObjectWithGivenProto(context, JS_GetClass(proto), proto);
JS::RootedObject object(context, JS_NewObjectWithGivenProto(
context, JS_GetClass(proto), proto));
if (!object)
return nullptr;
auto* priv = FundamentalInstance::new_for_js_object(context, object);
priv->associate_js_instance(object, gfundamental);
if (!priv->associate_js_instance(context, object, gfundamental))
return nullptr;
return object;
}
......
......@@ -145,7 +145,9 @@ class FundamentalInstance
const JS::CallArgs& args);
public:
void associate_js_instance(JSObject* object, void* gfundamental);
GJS_JSAPI_RETURN_CONVENTION
bool associate_js_instance(JSContext* cx, JSObject* object,
void* gfundamental);
};
G_BEGIN_DECLS
......
......@@ -33,7 +33,19 @@
#include "jsapi-util.h"
#include "jsapi-wrapper.h"
#include "js/GCHashTable.h"
#include "js/SweepingAPI.h"
using JobQueue = JS::GCVector<JSObject*, 0, js::SystemAllocPolicy>;
using FundamentalTable =
JS::GCHashMap<void*, JS::Heap<JSObject*>, js::DefaultHasher<void*>,
js::SystemAllocPolicy>;
// FundamentalTable's key type is void*, the GC sweep method should ignore it
namespace JS {
template <>
struct GCPolicy<void*> : public IgnoreGCPolicy<void*> {};
} // namespace JS
class GjsContextPrivate {
GjsContext* m_public_context;
......@@ -69,6 +81,9 @@ class GjsContextPrivate {
};
EnvironmentPreparer m_environment_preparer;
// Weak pointer mapping from fundamental native pointer to JSObject
JS::WeakCache<FundamentalTable>* m_fundamental_table;
uint8_t m_exit_code;
/* flags */
......@@ -122,6 +137,9 @@ class GjsContextPrivate {
GJS_USE bool is_owner_thread(void) const {
return m_owner_thread == g_thread_self();
}
GJS_USE JS::WeakCache<FundamentalTable>& fundamental_table(void) {
return *m_fundamental_table;
}
GJS_USE
static const GjsAtoms& atoms(JSContext* cx) { return from_cx(cx)->m_atoms; }
......
......@@ -342,6 +342,8 @@ void GjsContextPrivate::dispose(void) {
JS_BeginRequest(m_cx);
gjs_debug(GJS_DEBUG_CONTEXT, "Releasing cached JS wrappers");
m_fundamental_table->clear();
/* Do a full GC here before tearing down, since once we do
* that we may not have the JS_GetPrivate() to access the
* context
......@@ -372,6 +374,7 @@ void GjsContextPrivate::dispose(void) {
gjs_debug(GJS_DEBUG_CONTEXT, "Freeing allocated resources");
delete m_job_queue;
delete m_fundamental_table;
/* Tear down JS */
JS_DestroyContext(m_cx);
......@@ -451,6 +454,11 @@ GjsContextPrivate::GjsContextPrivate(JSContext* cx, GjsContext* public_context)
if (!m_job_queue)
g_error("Failed to initialize promise job queue");
JSRuntime* rt = JS_GetRuntime(m_cx);
m_fundamental_table = new JS::WeakCache<FundamentalTable>(rt);
if (!m_fundamental_table->init())
g_error("Failed to initialize fundamental objects table");
JS_BeginRequest(m_cx);
JS::RootedObject global(m_cx, gjs_create_global_object(m_cx));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment