THRIFT-3567 GLib-GObject-CRITICAL **: g_object_unref: assertion 'G_IS_OBJECT (object)' failed

Client: c_glib
Patch: Chandler May <cjmay4754@gmail.com>

This closes #806

4 files changed, 102 insertions, 7 deletions

diff --git a/lib/c_glib/test/CMakeLists.txt b/lib/c_glib/test/CMakeLists.txt
index 14454ed0d..411ae95fe 100644
--- a/lib/c_glib/test/CMakeLists.txt
+++ b/lib/c_glib/test/CMakeLists.txt
@@ -101,7 +101,12 @@ add_executable(testoptionalrequired testoptionalrequired.c)
 target_link_libraries(testoptionalrequired testgenc)
 add_test(NAME testoptionalrequired COMMAND testoptionalrequired)
 
-add_executable(testthrifttest testthrifttest.c)
+include_directories("${PROJECT_SOURCE_DIR}/test/c_glib/src" "${CMAKE_CURRENT_BINARY_DIR}/gen-c_glib")
+
+add_executable(testthrifttest testthrifttest.c
+    ${PROJECT_SOURCE_DIR}/test/c_glib/src/thrift_test_handler.c
+    ${PROJECT_SOURCE_DIR}/test/c_glib/src/thrift_test_handler.h
+    gen-c_glib/t_test_thrift_test_types.h)
 target_link_libraries(testthrifttest testgenc)
 add_test(NAME testthrifttest COMMAND testthrifttest)
 
diff --git a/lib/c_glib/test/Makefile.am b/lib/c_glib/test/Makefile.am
index 43f3b4b81..e533c41b0 100755
--- a/lib/c_glib/test/Makefile.am
+++ b/lib/c_glib/test/Makefile.am
@@ -161,8 +161,12 @@ testoptionalrequired_LDADD = \
     $(top_builddir)/lib/c_glib/src/thrift/c_glib/transport/libthrift_c_glib_la-thrift_transport.o \
     libtestgenc.la
 
-testthrifttest_SOURCES = testthrifttest.c
+testthrifttest_SOURCES = testthrifttest.c \
+    $(top_srcdir)/test/c_glib/src/thrift_test_handler.c \
+    $(top_srcdir)/test/c_glib/src/thrift_test_handler.h \
+    gen-c_glib/t_test_thrift_test_types.h
 testthrifttest_LDADD = libtestgenc.la
+testthrifttest_CFLAGS = -I$(top_srcdir)/test/c_glib/src -I./gen-c_glib $(GLIB_CFLAGS)
 
 testthrifttestclient_SOURCES = testthrifttestclient.cpp
 testthrifttestclient_CPPFLAGS = -I../../cpp/src $(BOOST_CPPFLAGS) -I./gen-cpp -I../src -I./gen-c_glib $(GLIB_CFLAGS)
diff --git a/lib/c_glib/test/testthrifttest.c b/lib/c_glib/test/testthrifttest.c
index 5f0f6e33b..0211f5393 100755
--- a/lib/c_glib/test/testthrifttest.c
+++ b/lib/c_glib/test/testthrifttest.c
@@ -1,9 +1,13 @@
 #include <assert.h>
 #include <netdb.h>
 
+#include <thrift/c_glib/thrift.h>
 #include <thrift/c_glib/transport/thrift_server_transport.h>
 #include <thrift/c_glib/transport/thrift_server_socket.h>
 
+#include "t_test_thrift_test_types.h"
+#include "thrift_test_handler.h"
+
 static const char TEST_ADDRESS[] = "localhost";
 static const int TEST_PORT = 64444;
 
@@ -16,6 +20,83 @@ test_thrift_server (void)
   g_object_unref (tsocket);
 }
 
+static void
+set_indicator (gpointer data, GObject *where_the_object_was) {
+  THRIFT_UNUSED_VAR(where_the_object_was);
+
+  *(gboolean *) data = TRUE;
+}
+
+static void
+test_thrift_handler (void)
+{
+  GError *error;
+  GHashTable *_return;
+  TTestInsanity *argument;
+  gboolean indicator;
+
+  TTestXtruct  *xtruct,  *xtruct2;
+  TTestNumberz numberz;
+  TTestNumberz numberz2;
+  TTestUserId user_id, *user_id_ptr, *user_id_ptr2;
+  GHashTable *user_map;
+  GPtrArray *xtructs;
+
+  error = NULL;
+  indicator = FALSE;
+
+  user_map = NULL;
+  xtructs = NULL;
+
+  argument = g_object_new (T_TEST_TYPE_INSANITY, NULL);
+  g_object_get (argument,
+                "userMap", &user_map,
+                "xtructs", &xtructs,
+                NULL);
+
+  numberz = T_TEST_NUMBERZ_FIVE;
+  numberz2 = T_TEST_NUMBERZ_EIGHT;
+  user_id_ptr = g_malloc (sizeof *user_id_ptr);
+  *user_id_ptr = 5;
+  user_id_ptr2 = g_malloc (sizeof *user_id_ptr);
+  *user_id_ptr2 = 8;
+  g_hash_table_insert (user_map, (gpointer)numberz, user_id_ptr);
+  g_hash_table_insert (user_map, (gpointer)numberz2, user_id_ptr2);
+  g_hash_table_unref (user_map);
+
+  xtruct = g_object_new (T_TEST_TYPE_XTRUCT,
+                         "string_thing", "Hello2",
+                         "byte_thing",   2,
+                         "i32_thing",    2,
+                         "i64_thing",    2LL,
+                         NULL);
+  xtruct2 = g_object_new (T_TEST_TYPE_XTRUCT,
+                          "string_thing", "Goodbye4",
+                          "byte_thing",   4,
+                          "i32_thing",    4,
+                          "i64_thing",    4LL,
+                          NULL);
+  g_ptr_array_add (xtructs, xtruct2);
+  g_ptr_array_add (xtructs, xtruct);
+  g_ptr_array_unref (xtructs);
+
+  _return = g_hash_table_new_full (g_int64_hash,
+                                   g_int64_equal,
+                                   g_free,
+                                   (GDestroyNotify)g_hash_table_unref);
+
+  g_object_weak_ref (G_OBJECT (argument), set_indicator, (gpointer) &indicator);
+
+  assert (thrift_test_handler_test_insanity (NULL, &_return, argument, &error));
+  assert (! indicator);
+
+  g_hash_table_unref (_return);
+  assert (! indicator);
+
+  g_object_unref (argument);
+  assert (indicator);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -26,6 +107,7 @@ main(int argc, char *argv[])
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/testthrift/Server", test_thrift_server);
+  g_test_add_func ("/testthrift/Handler", test_thrift_handler);
 
   return g_test_run ();
 }
diff --git a/test/c_glib/src/thrift_test_handler.c b/test/c_glib/src/thrift_test_handler.c
index 8312ad9b8..1d8bcb25a 100644
--- a/test/c_glib/src/thrift_test_handler.c
+++ b/test/c_glib/src/thrift_test_handler.c
@@ -520,11 +520,15 @@ thrift_test_handler_test_insanity (TTestThriftTestIf    *iface,
                        GINT_TO_POINTER (T_TEST_NUMBERZ_THREE),
                        (gpointer)argument);
 
-  /* Increment argument's ref count since first_map now holds two
-     references to it and would otherwise attempt to deallocate it
-     twice during destruction. We do this instead of creating a copy
-     of argument in order to mimic the C++ implementation (and since,
-     frankly, the world needs less argument, not more). */
+  /* Increment argument's ref count by two because first_map now holds
+     two references to it and the caller is not aware we have made any
+     additional references to argument.  (That is, caller owns argument
+     and will unref it explicitly in addition to unref-ing *_return.)
+
+     We do this instead of creating a copy of argument in order to mimic
+     the C++ implementation (and since, frankly, the world needs less
+     argument, not more). */
+  g_object_ref ((gpointer)argument);
   g_object_ref ((gpointer)argument);
 
   looney = g_object_new (T_TEST_TYPE_INSANITY, NULL);