Comment 1 Anatoly Spektor 2012-07-09 16:41:53 EDT

Please ignore patch above, new patch will be submitted shortly. Sorry for this confusion.

Comment 2 Anatoly Spektor 2012-07-10 10:32:59 EDT

This patch omits use of gtk_object_sink() and uses g_object_ref_sink() instead:

http://fedorapeople.org/gitweb?p=aspektor/public_git/eclipse.platform.swt.git;a=commit;h=8d78e57a9d36dde24d0137b69eac0326bca95ebc

Please use this patch instead of one posted in the first comment.

Regards,

Anatoly

Comment 3 Silenio Quarti 2012-08-01 13:54:37 EDT

The version check is wrong. The function g_object_ref_sink() belongs to GLIB (not GTK). There two options to fix this:

1) Add OS.GLIB_VERSION()
2) Check for the GTK version that corresponds to GLIB 2.10.

Option 1) is cleaner.

Please add a helper function as well (the same way I added for bug#384651).

Comment 4 Anatoly Spektor 2012-08-02 15:20:07 EDT

Ok, here is revised patch implementing option 1 and helper function:

http://fedorapeople.org/cgit/aspektor/public_git/eclipse.platform.swt.git/commit/?h=g_object_ref_sink


Please let me know if there are some other concerns regarding this patch.

Regards,

Anatoly

Comment 5 Silenio Quarti 2012-08-03 10:52:13 EDT

Thanks

Pushed to eclipse.org

http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=34f0c3974a377d1fb41f2905de1ceb16ac6ba1ce

Comment 6 Arun Thondapu 2012-08-03 11:34:36 EDT

Just a quick question on this patch - should the #define be
#define g_object_ref_sink_LIB LIB_GLIB instead of
#define g_object_ref_sink_LIB LIB_GIO or it really does not matter since both libraries are bundled together anyway?

Comment 7 Silenio Quarti 2012-08-03 12:29:37 EDT

(In reply to comment #6)
> Just a quick question on this patch - should the #define be
> #define g_object_ref_sink_LIB LIB_GLIB instead of
> #define g_object_ref_sink_LIB LIB_GIO or it really does not matter since
> both libraries are bundled together anyway?

Arun, that is a very good question and the short answer is neither: it should be LIB_GOBJECT. 

Before I released the first patch I hacked the C code to make sure the dynamic loaded functions were found. We need to do that because the failure case is silent and in this case we would be leaking without knowing. The funny thing is that g_object_ref_sink is found when using LIB_GIO (probably because GIO depends on GOBJECT). It is not found with LIB_GLIB.

I fixed the code to look up g_object_ref_sink from GOBJECT.

http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=165a14baa4f28638048d0b616f73e1b86413e4a2

Comment 8 Arun Thondapu 2012-08-03 12:56:19 EDT

Thanks for the clarification Silenio!
My actual question was whether it makes sense to add LIB_GOBJECT for g_object_ref_sink but then I assumed it may not be necessary since we can find it via LIB_GIO or LIB_GLIB and didn't ask.
Anyway, it makes more sense to me now.

Comment 9 Arun Thondapu 2012-08-07 04:08:57 EDT

Just realized there is another clarification here, we're now referring to g_object_ref_sink from LIB_GOBJECT but the version check we're using is based on GLIB_VERSION. This would still work though, because they use the same version numbers, may be its better to add a code comment to be clear about the same?