[Webkit-unassigned] [Bug 27980] Give an ability to WebKit to free statically allocated pointers before quit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27980





--- Comment #12 from Zoltan Herczeg <zherczeg at inf.u-szeged.hu>  2009-08-05 06:45:06 PDT ---
Unfortunately there are several ways to define global variables, I don't think
it is possible to merge them into a single preprocessor define:
 - using DEFINE_STATIC_LOCAL
   * creates static global variables inside a function
   * the object is created before the first call of the function
 - using DEFINE_GLOBAL (WebCore/platform/StaticConstructors.h)
   * these variables cannot be freed, since they are allocated on the heap
   * workaround: wrapper classes
     Any better idea?
   * fortunately it is a rarely used directive (only two classes: AtomicString
and QualifiedName)
 - Plain global variables
   [static] class* [class_name::]variable;
   * I will try to use a static alayzer tool to find these objects
   * my propose:
     use '[static] WTF::StaticPtr<class> [class_name::]variable;' instead of
them in the future

WTF::StaticPtr similar to OwnPtr, except:
  - only one object can be assigned to it. The object can be passed to the
constructor, or assigned later.

> Is the goal here to make memory leak debugging tools work better? Or is there
> value beyond that?

Ok, you win here :) tonikitoo and ddkilzer mentioned they could use this
feature. I also plan to use it for embedded devices, but first I would like to
know whether it is possible to do it. But right know they are just plans,
nothing more.

> Could you write a document about how to use StaticPtr correctly? I'd like to
> review that to get an idea how we would make sure we were using it properly on
> various platforms. Would we consider it a coding style violation to make a
> global that doesn't use StaticPtr?

"WTF::StaticPtr<class>" would be a substitution of "class*", and it would mimic
the default C++ pointer behaviour as possible (with operator overloading). I
hope "class&" definitions are rare except for DEFINE_STATIC_LOCAL.

> As part of this are you proposing that DEFINE_STATIC_LOCAL be the only legal
> way to make global variables?

No. DEFINE_STATIC_LOCAL has a special purpose, and looks like not cover all
global variables.

> I do not want to sign up everyone on the project to write a lot of code to
> deallocate every last object if this code isn't actually useful to someone on
> one of the ports. I need to hear more justification about why to take on all
> this additional complexity. Making all the global data structures practical to
> delete is an enormous task that this patch barely scratches the surface of.

Well I think a global policy can be:
  - either do not write destructors (someone else can do later)
  - or write a working, and tested constructor.

> > +#if !PLATFORM(QT)
> >      delete data;
> > +#endif

This delete cause a segmentation fault in Qt. It has not appeared until now,
since this destructor is never called. (Although this code may have worked,
when the destructor was implemented, but it was not covered by regression
tests, and has gone wrong eventually).

I think if we write a destructor, we have to maintain it as the other codes,
and should cover them in regression tests. This may be a good aim for this
project.

I have fixed the typos and other things (the looks much better now), except:

> > +    PtrType operator=(PtrType);
> 
> This seems like bad style. Since values stored in this pointer will be freed, I
> think we want this to use some explicit style like OwnPtr rather than plain
> assignment.

Could you show me an example here?

Anyway, a plain QtLauncher now frees 761 objects (without crash). After some
browsing it grows to 812.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.