Bug 83752

Summary: [GTK] Use GRefPtr to delete the default web context in WebKit2 GTK+
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gns, mrobinson, pnormand, webkit.review.bot
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Use GRefPtr instead of atexit mrobinson: review+

Description Carlos Garcia Campos 2012-04-12 00:45:36 PDT
Default web context is global and never freed. We can use atexit() like we do in webkit1 to make usre it's finalized before the process finishes.
Comment 1 Carlos Garcia Campos 2012-04-12 00:48:51 PDT
Created attachment 136839 [details]
Patch
Comment 2 WebKit Review Bot 2012-04-12 00:52:16 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Martin Robinson 2012-04-12 08:12:41 PDT
Comment on attachment 136839 [details]
Patch

It might be cleaner to use GRefPtr and DECLARE_STATIC_LOCAL
Comment 4 Carlos Garcia Campos 2012-04-12 23:59:51 PDT
(In reply to comment #3)
> (From update of attachment 136839 [details])
> It might be cleaner to use GRefPtr and DECLARE_STATIC_LOCAL

That doesn't help, we want to make sure finalize is called before the process finishes. Static variables declared with DEFINE_STATIC_LOCAL are leaked, the destructor of the GRefPtr is not called at exit.
Comment 5 Carlos Garcia Campos 2012-04-13 00:10:12 PDT
Created attachment 137049 [details]
Use GRefPtr instead of atexit
Comment 6 Martin Robinson 2012-04-13 08:04:33 PDT
Comment on attachment 137049 [details]
Use GRefPtr instead of atexit

Thanks!
Comment 7 Martin Robinson 2012-04-13 08:08:04 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 136839 [details] [details])
> > It might be cleaner to use GRefPtr and DECLARE_STATIC_LOCAL
> 
> That doesn't help, we want to make sure finalize is called before the process finishes. Static variables declared with DEFINE_STATIC_LOCAL are leaked, the destructor of the GRefPtr is not called at exit.

From what I understand the point of DEFINE_STATIC_LOCAL is to work around a compiler bug to ensure that the variables contained are cleaned up at exit. If that's not happening it seems like a bug in DEFINE_STATIC_LOCAL. So GRefPtr may work here, but perhaps not for all compilers.
Comment 8 Carlos Garcia Campos 2012-04-13 08:32:02 PDT
(In reply to comment #7)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (From update of attachment 136839 [details] [details] [details])
> > > It might be cleaner to use GRefPtr and DECLARE_STATIC_LOCAL
> > 
> > That doesn't help, we want to make sure finalize is called before the process finishes. Static variables declared with DEFINE_STATIC_LOCAL are leaked, the destructor of the GRefPtr is not called at exit.
> 
> From what I understand the point of DEFINE_STATIC_LOCAL is to work around a compiler bug to ensure that the variables contained are cleaned up at exit. If that's not happening it seems like a bug in DEFINE_STATIC_LOCAL. So GRefPtr may work here, but perhaps not for all compilers.

// Use these to declare and define a static local variable (static T;) so that                                                                                                               
//  it is leaked so that its destructors are not called at exit. Using this                                                                                                                  
//  macro also allows workarounds a compiler bug present in Apple's version of GCC 4.0.1.
Comment 9 Carlos Garcia Campos 2012-04-15 23:38:02 PDT
Committed r114226: <http://trac.webkit.org/changeset/114226>