Bug 83752

Summary:

[GTK] Use GRefPtr to delete the default web context in WebKit2 GTK+

Product:

WebKit

Reporter:

Carlos Garcia Campos <cgarcia>

Component:

WebKit2

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gustavo, mrobinson, pnormand, webkit.review.bot

Priority:

Keywords:

Gtk

Version:

528+ (Nightly build)

Hardware:

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>