View source for Memory Management for nsIScriptContext

=Script Object Lifetime Management=

The problem:

Certain nsIScriptContext methods return a |void *| tied to the specific language.  For example, |CompileEventHandler| returns a "function" object which is held by the caller and later passed to |CallEventHandler|.  The lifetime of these returned |void *| objects must be managed.

At the high-level, it has been agreed that new methods |DropScriptObject| and |HoldScriptObject| will be added to certain language-specific interfaces.  The affected |nsIScriptContext| methods will return a 'held' object; the caller must call |DropScriptObject| when it no longer needs to store it.  |HoldScriptObject| is provided so that copies of the objects can be made.  This is identical to a reference counting scheme, but named to avoid confusion with XPCOMs reference counting.

We need to turn this into an actual implementation, with the following restrictions:

* We want to avoid additional memory allocation or virtual method calls.  The only overhead the Drop/Hold scheme requires is one additional virtual-method call for the memory management function itself.

* We can not have the memory management functions exclusively on nsIScriptContext.  There is code that currently stores a |void *| script object, but has no nsIScriptContext available.  One example is nsJSEventListener, which stores the |void *|, but has the |nsIScriptContext| provided as part of invoking the event.  nsJSEventListener would not want to store an nsIScriptContext just to free the script object.  Another example is the XUL cache which will be extended to cache all languages, but will not want to store an nsIScriptContext just to handle cache shutdown.

* Linkage issues mean that "helper functions" are unsuitable for use here.  As the DOM implementation and each language implementation are individual components, public static functions are not globally reachable.  Either xpcom interfaces or fully inline C++ classes can be used to assist with these tasks.

== Implementation idea #1 ==

Add DropScriptObect/HoldScriptObject to nsILanguageRuntime (a new class - a 'singleton' for each language - it creates nsIScriptContext objects amongst other things).  Code then only needs an integer language ID to be able to free the script object.

A downside of this approach is that all memory management functions take an extra penalty.  Each function involves:
* Fetching the nsDOMScriptObjectFactory service.
* Fetching the nsILanguageRuntime from the nsDOMScriptObjectFactory passing the int langID.
* Calling the nsILanguageRuntime::Drop/HoldScriptObject function.

This problem could be solved by either:
* Duplicating these functions on nsIScriptContext, so code that does have a context available can do so without the additional overhead.
* Having performance critical code keep a reference to the nsILanguageRuntime.

Even still, a key problem to this approach remains its fragility.  By explicitly requiring manual Drop/Hold calls, there is alot of scope for programmer error in the face of error conditions (ie, early returns) to cause difficult to track leaks.  The inevitability of such errors is evidenced by nsCOMPtr and various other tricks already in the code base.

== Implementation idea #2 ==

This implementation builds on #1 as its core framework, and attempts to add a "helper class" (nsScriptObjectHolder) to automate the management of these objects.  All nsIScriptContext methods that return a "new" script object now take a reference to one of these objects.

The public prototype for this class is:

 class nsScriptObjectHolder {
 public:
    nsScriptObjectHolder();
    nsScriptObjectHolder(PRUint32 aLangID, void *aObject);
    nsScriptObjectHolder(const nsScriptObjectHolder& other);
    ~nsScriptObjectHolder();
    nsScriptObjectHolder &operator=(const nsScriptObjectHolder &other);
    PRBool operator!() const;
    operator void *() const;
    nsresult set(PRUint32 langID, void *object);
    nsresult set(void *object);
    nsresult set(const nsScriptObjectHolder &other);
    nsresult clear();
    PRUint32 mLangID;
 };

the nsIScriptContext methods changed are CompileScript, CompileEventHandler, GetBoundEventHandler, CompileFunction and Deserialize, with all changes similar to:

    virtual nsresult GetBoundEventHandler(nsISupports* aTarget, void *aScope,
                                          nsIAtom* aName,
 -                                        void** aHandler) = 0;
 +                                        nsScriptObjectHolder &aHandler) = 0;


Code calling these methods changes from:
  void *handler = nsnull;
  rv = context->GetBoundEventHandler(..., &handler);
  if (handler)
    context->CallEventHandler(..., handler)

to:

  nsScriptObjectHolder handler;
  rv = context->GetBoundEventHandler(..., handler);
  if (handler)
    context->CallEventHandler(..., handler)

The use of "operator void *" and "operator !" means that much existing code that uses |void *| can remain unchanged once the variable has changed to nsScriptObjectHolder.  For example, an nsScriptObjectHolder can be passed to nsIScriptContext::CallEventHandler, even though the prototype remains |void *|.

=== Problems ===

* The nsScriptObjectHolder class must have a fully inline implementation.  Any attempt to use methods defined in gklayout.dll mean the class can not be used by external languages (such as Python).

* This does not lend itself to allowing DropScriptObject/HoldScriptObject on both nsIScriptContext and nsILanguageRuntime.  As not all callers can use nsIScriptContext, it must be nsILanguageRuntime.  This means each memory management function is slightly expensive, as detailed above.

== Implementation idea #3 ==

Use a real XPCOM interface.  Something like:

 interface nsIScriptObjectHolder : public nsISupports
 {
    virtual void *GetScriptObject();
 }

and all nsIScriptContext methods return one of these interfaces instead of a |void *|.  The memory management is then implicitly tied up in the lifetime of the interface itself.

=== Problems ===
* Fetching the |void *| language object becomes a virtual method.
* Each call requires a new allocation for the interface.

== Notes ==

Implementation idea #2 has been implemented and works well.  However, concerns exist regarding the performace overhead in terms of speed and bloat.