Changeset 13568 in webkit


Ignore:
Timestamp:
Mar 29, 2006 6:39:24 PM (18 years ago)
Author:
ggaren
Message:

JavaScriptCore:

Reviewed by Darin.

  • JavaScriptCore side of fix for <rdar://problem/4308243> 8F36 Regression: crash in malloc_consolidate if you use a .PAC file

The crash was a result of threaded deallocation of thread-unsafe
objects. Pure JS objects are thread-safe because all JS execution
is synchronized through JSLock. However, JS objects that wrap WebCore
objects are thread-unsafe because JS and WebCore execution are not
synchronized. That unsafety comes into play when the collector
deallocates a JS object that wraps a WebCore object, thus causing the
WebCore object to be deallocated.

The solution here is to have each JSCell know whether it is safe to
collect on a non-main thread, and to avoid collecting unsafe cells
when on a non-main thread.

We don't have a way to test PAC files yet, so there's no test
attached to this patch.

  • kjs/collector.cpp: (KJS::Collector::collect):
(1) Added the test "currentThreadIsMainThread

imp->m_destructorIsThreadSafe".

  • kjs/protect.h: (KJS::gcProtectNullTolerant): (KJS::gcUnprotectNullTolerant):
  • kjs/value.h: (KJS::JSCell::JSCell): The bools here must be bitfields, otherwise m_destructorIsThreadSafe becomes another whole word, ruining the collector optimizations we've made based on the size of a JSObject.
  • kxmlcore/FastMalloc.cpp: (KXMLCore::currentThreadIsMainThread): (KXMLCore::fastMallocRegisterThread):
  • kxmlcore/FastMalloc.h:

WebCore:

Reviewed by Hyatt.

  • css/html4.css: Added default style info for new text fields.
  • rendering/RenderTextField.cpp: (WebCore::RenderTextField::createDivStyle): Added an extra 1px of padding on the left & right to match Win IE & the latest Mozilla. (WebCore::RenderTextField::updateFromElement): Removed some outdated comments. Cleaned up the way we add text nodes to the div. (WebCore::RenderTextField::setSelectionStart): Tweaked selection code to better match Mozilla behavior. (WebCore::RenderTextField::setSelectionEnd): ditto. (WebCore::RenderTextField::select): Cleaned this up by having it call setSelectionRange. (WebCore::RenderTextField::setSelectionRange): Calls updateLayout now in case this is called in an onload handler, and no other layout has occurred. (WebCore::RenderTextField::calcMinMaxWidth): Use floatWidth to calculate the width of the "0" character.
  • rendering/RenderTheme.cpp: (WebCore::RenderTheme::isControlStyled): If the text field's specified border is different from the default border, then treat the control as styled, so the engine knows to turn off the aqua appearance.
  • rendering/RenderThemeMac.mm: (WebCore::RenderThemeMac::paintTextField): return false so the engine knows not to try to draw the border. (WebCore::RenderThemeMac::adjustTextFieldStyle): text field style info has been moved to html4.css. We also add intrinsic margins here if the font size is large enough.
  • html/HTMLTextFieldInnerElement.cpp: (WebCore::HTMLTextFieldInnerElement::defaultEventHandler): No longer check for appearance. All text fields with m_type == TEXT will use the new implementation.
  • html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::isKeyboardFocusable): ditto. (WebCore::HTMLInputElement::focus): ditto. (WebCore::HTMLInputElement::selectionStart): ditto. (WebCore::HTMLInputElement::selectionEnd): ditto. (WebCore::HTMLInputElement::setSelectionStart): ditto. (WebCore::HTMLInputElement::setSelectionEnd): ditto. (WebCore::HTMLInputElement::select): ditto. (WebCore::HTMLInputElement::setSelectionRange): ditto. (WebCore::HTMLInputElement::createRenderer): ditto. (WebCore::HTMLInputElement::defaultEventHandler): ditto. (WebCore::HTMLInputElement::isMouseFocusable): Added. Old text fields relied on the widget to provide a focus policy. A text field that is focusable should be mouse focusable, and shouldn't need to ask the base class.
  • html/HTMLInputElement.h: Added isMouseFocusable.
  • html/HTMLGenericFormElement.cpp: (WebCore::HTMLGenericFormElement::isMouseFocusable): Removed specific text field code since that is now done in HTMLInputElement::isMouseFocusable.
  • dom/Document.cpp: (WebCore::Document::clearSelectionIfNeeded): Check that the new selection is does not have a shadowAncestorNode that is focused.
Location:
trunk
Files:
20 edited

Legend:

Unmodified
Added
Removed
  • trunk/JavaScriptCore/ChangeLog

    r13541 r13568  
     12006-03-29  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Darin.
     4
     5        - JavaScriptCore side of fix for <rdar://problem/4308243> 8F36
     6        Regression: crash in malloc_consolidate if you use a .PAC file
     7
     8        The crash was a result of threaded deallocation of thread-unsafe
     9        objects. Pure JS objects are thread-safe because all JS execution
     10        is synchronized through JSLock. However, JS objects that wrap WebCore
     11        objects are thread-unsafe because JS and WebCore execution are not
     12        synchronized.  That unsafety comes into play when the collector
     13        deallocates a JS object that wraps a WebCore object, thus causing the
     14        WebCore object to be deallocated.
     15
     16        The solution here is to have each JSCell know whether it is safe to
     17        collect on a non-main thread, and to avoid collecting unsafe cells
     18        when on a non-main thread.
     19
     20        We don't have a way to test PAC files yet, so there's no test
     21        attached to this patch.
     22
     23        * kjs/collector.cpp:
     24        (KJS::Collector::collect):
     25        (1) Added the test "currentThreadIsMainThread ||
     26        imp->m_destructorIsThreadSafe".
     27
     28        * kjs/protect.h:
     29        (KJS::gcProtectNullTolerant):
     30        (KJS::gcUnprotectNullTolerant):
     31        * kjs/value.h:
     32        (KJS::JSCell::JSCell): The bools here must be bitfields, otherwise
     33        m_destructorIsThreadSafe becomes another whole word, ruining the
     34        collector optimizations we've made based on the size of a JSObject.
     35        * kxmlcore/FastMalloc.cpp:
     36        (KXMLCore::currentThreadIsMainThread):
     37        (KXMLCore::fastMallocRegisterThread):
     38        * kxmlcore/FastMalloc.h:
     39
    1402006-03-28  Darin Adler  <darin@apple.com>
    241
  • trunk/JavaScriptCore/kjs/collector.cpp

    r13089 r13568  
    444444    InterpreterImp *scr = InterpreterImp::s_hook;
    445445    do {
    446       //fprintf( stderr, "Collector marking interpreter %p\n",(void*)scr);
    447446      scr->mark();
    448447      scr = scr->next;
     
    461460  size_t numLiveObjects = heap.numLiveObjects;
    462461
     462#if USE(MULTIPLE_THREADS)
     463  bool currentThreadIsMainThread = !pthread_is_threaded_np() || pthread_main_np();
     464#else
     465  bool currentThreadIsMainThread = true;
     466#endif
     467 
    463468  for (size_t block = 0; block < heap.usedBlocks; block++) {
    464469    CollectorBlock *curBlock = heap.blocks[block];
     
    474479        if (imp->m_marked) {
    475480          imp->m_marked = false;
    476         } else {
     481        } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
    477482          imp->~JSCell();
    478483          --usedCells;
     
    495500          if (imp->m_marked) {
    496501            imp->m_marked = false;
    497           } else {
     502          } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
    498503            imp->~JSCell();
    499504            --usedCells;
  • trunk/JavaScriptCore/kjs/object.h

    r13465 r13568  
    118118     * @param proto The prototype
    119119     */
    120     JSObject(JSObject *proto);
     120    JSObject(JSObject *proto, bool destructorIsThreadSafe = true);
    121121
    122122    /**
     
    124124     * (that is, the ECMAScript "null" value, not a null object pointer).
    125125     */
    126     JSObject();
     126    explicit JSObject(bool destructorIsThreadSafe = true);
    127127
    128128    virtual void mark();
     
    580580JSObject *throwError(ExecState *, ErrorType);
    581581
    582 inline JSObject::JSObject(JSObject *proto)
    583     : _proto(proto), _internalValue(0)
     582inline JSObject::JSObject(JSObject *proto, bool destructorIsThreadSafe)
     583    : JSCell(destructorIsThreadSafe)
     584    , _proto(proto)
     585    , _internalValue(0)
    584586{
    585587    assert(proto);
    586588}
    587589
    588 inline JSObject::JSObject()
    589     : _proto(jsNull()), _internalValue(0)
     590inline JSObject::JSObject(bool destructorIsThreadSafe)
     591    : JSCell(destructorIsThreadSafe)
     592    , _proto(jsNull())
     593    , _internalValue(0)
    590594{
    591595}
  • trunk/JavaScriptCore/kjs/protect.h

    r12301 r13568  
    4444    inline void gcProtectNullTolerant(JSValue *val)
    4545    {
    46         if (val) gcProtect(val);
     46        if (val)
     47            gcProtect(val);
    4748    }
    4849
    4950    inline void gcUnprotectNullTolerant(JSValue *val)
    5051    {
    51         if (val) gcUnprotect(val);
     52        if (val)
     53            gcUnprotect(val);
    5254    }
    5355   
  • trunk/JavaScriptCore/kjs/value.h

    r12949 r13568  
    122122    friend class GetterSetterImp;
    123123private:
    124     JSCell();
     124    explicit JSCell(bool destructorIsThreadSafe = true);
    125125    virtual ~JSCell();
    126126public:
     
    156156
    157157private:
    158     bool m_marked;
     158    bool m_destructorIsThreadSafe : 1;
     159    bool m_marked : 1;
    159160};
    160161
     
    209210}
    210211
    211 inline JSCell::JSCell()
    212     : m_marked(false)
     212inline JSCell::JSCell(bool destructorIsThreadSafe)
     213    : m_destructorIsThreadSafe(destructorIsThreadSafe)
     214    , m_marked(false)
    213215{
    214216}
  • trunk/JavaScriptCore/kxmlcore/FastMalloc.cpp

    r13091 r13568  
    104104#endif
    105105
    106 } // namespace KJS
     106} // namespace KXMLCore
    107107
    108108#else
     
    13961396    if (thread != mainThreadID) {
    13971397        // We lock when writing isMultiThreaded but not when reading it.
    1398         // It's ok if the main thread temporarily gets it wrong - the global
    1399         // variable cache is the same as the thread-specific cache for the main thread.
     1398        // It's ok if the main thread gets it wrong - for the main thread, the
     1399        // global variable cache is the same as the thread-specific cache.
    14001400        // And other threads can't get it wrong because they must have gone through
    14011401        // this function before allocating so they've synchronized.
    14021402        // Also, mainThreadCache is only set when isMultiThreaded is false,
    1403         // to save a branchin some cases.
     1403        // to save a branch in some cases.
    14041404        SpinLockHolder lock(&multiThreadedLock);
    14051405        isMultiThreaded = true;
     
    23162316
    23172317#if KXC_CHANGES
    2318 } // namespace KJS
     2318} // namespace KXMLCore
    23192319#endif
    23202320
  • trunk/JavaScriptCore/kxmlcore/FastMalloc.h

    r13089 r13568  
    3333    void fastFree(void* p);
    3434    void *fastRealloc(void* p, size_t n);
    35    
     35
    3636} // namespace KXMLCore
    3737
  • trunk/WebCore/ChangeLog

    r13567 r13568  
    7676        (WebCore::Image::getNSImage): Use KWQRetainNSRelease to properly retain
    7777        a reference to an Objective-C object in a C++ class.
     78
     792006-03-29  Geoffrey Garen  <ggaren@apple.com>
     80
     81        Reviewed by Darin.
     82
     83        - WebCore side of fix for <rdar://problem/4308243> 8F36 Regression:
     84        crash in malloc_consolidate if you use a .PAC file
     85
     86        (1) To ensure thread-safe deallocation, set the "unsafe to destroy on
     87        non-main threads" bit in the DOMObject constructor.
     88
     89        (2) Made all binding objects inherit from DOMObject, because the
     90        WebCore data structures they wrap are not thread-safe. "DOMObject" is
     91        a slightly awkward name for things like the Window object, but the
     92        DOM spec is considering adding a Window object, and creating a whole
     93        new base class for this purpose seemed like overkill.
     94
     95        * khtml/ecma/JSDOMParser.h:
     96        * khtml/ecma/JSXMLHttpRequest.h:
     97        * khtml/ecma/JSXMLSerializer.cpp:
     98        (KJS::XMLSerializerConstructorImp::XMLSerializerConstructorImp):
     99        * khtml/ecma/JSXMLSerializer.h:
     100        * khtml/ecma/JSXSLTProcessor.h:
     101        * khtml/ecma/kjs_binding.h:
     102        (KJS::DOMObject::DOMObject): Unset the "safe to collect on non-main
     103        threads bit" to ensure thread-safe deallocation.
     104        * khtml/ecma/kjs_html.h:
     105        * khtml/ecma/kjs_navigator.cpp:
     106        (KJS::Navigator::Navigator):
     107        (KJS::PluginBase::PluginBase):
     108        * khtml/ecma/kjs_navigator.h:
     109        * khtml/ecma/kjs_proxy.cpp:
     110        * khtml/ecma/kjs_window.cpp:
     111        (KJS::History::History):
     112        (KJS::FrameArray::FrameArray):
     113        (KJS::Screen::Screen):
     114        (KJS::Window::Window):
     115        (KJS::BarInfo::BarInfo):
     116        * khtml/ecma/kjs_window.h:
    78117
    791182006-03-29  Geoffrey Garen  <ggaren@apple.com>
  • trunk/WebCore/khtml/ecma/JSDOMParser.h

    r13393 r13568  
    2727namespace KJS {
    2828
    29   class DOMParserConstructorImp : public JSObject {
     29  class DOMParserConstructorImp : public DOMObject {
    3030  public:
    3131    DOMParserConstructorImp(ExecState*, WebCore::Document*);
  • trunk/WebCore/khtml/ecma/JSXMLHttpRequest.h

    r13393 r13568  
    3333namespace KJS {
    3434
    35   class JSXMLHttpRequestConstructorImp : public JSObject {
     35  class JSXMLHttpRequestConstructorImp : public DOMObject {
    3636  public:
    3737    JSXMLHttpRequestConstructorImp(ExecState *exec, WebCore::Document *d);
  • trunk/WebCore/khtml/ecma/JSXMLSerializer.cpp

    r13393 r13568  
    4242
    4343XMLSerializerConstructorImp::XMLSerializerConstructorImp(ExecState *exec)
    44     : JSObject()
    4544{
    4645    putDirect(prototypePropertyName, XMLSerializerProto::self(exec), DontEnum|DontDelete|ReadOnly);
  • trunk/WebCore/khtml/ecma/JSXMLSerializer.h

    r13393 r13568  
    2828  class JSEventListener;
    2929
    30   class XMLSerializerConstructorImp : public JSObject {
     30  class XMLSerializerConstructorImp : public DOMObject {
    3131  public:
    3232    XMLSerializerConstructorImp(ExecState *);
  • trunk/WebCore/khtml/ecma/JSXSLTProcessor.h

    r13393 r13568  
    5858};
    5959
    60 class XSLTProcessorConstructorImp : public JSObject {
     60class XSLTProcessorConstructorImp : public DOMObject {
    6161public:
    6262    XSLTProcessorConstructorImp(ExecState *);
  • trunk/WebCore/khtml/ecma/kjs_binding.h

    r13405 r13568  
    4747  class DOMObject : public JSObject {
    4848  protected:
    49     DOMObject() : JSObject() {}
     49      // DOMObject Destruction is not thread-safe because JS DOM objects
     50      // wrap unsafe WebCore DOM data structures
     51      DOMObject() : JSObject(false) {}
    5052  public:
    51     virtual UString toString(ExecState *exec) const;
     53      virtual UString toString(ExecState *exec) const;
    5254  };
    5355
  • trunk/WebCore/khtml/ecma/kjs_html.h

    r13393 r13568  
    340340  ////////////////////// Option Object ////////////////////////
    341341
    342   class OptionConstructorImp : public JSObject {
     342  class OptionConstructorImp : public DOMObject {
    343343  public:
    344344    OptionConstructorImp(ExecState *exec, WebCore::Document *d);
     
    351351  ////////////////////// Image Object ////////////////////////
    352352
    353   class ImageConstructorImp : public JSObject {
     353  class ImageConstructorImp : public DOMObject {
    354354  public:
    355355    ImageConstructorImp(ExecState *exec, WebCore::Document *d);
  • trunk/WebCore/khtml/ecma/kjs_navigator.cpp

    r13410 r13568  
    3737namespace KJS {
    3838
    39     class PluginBase : public JSObject {
     39    class PluginBase : public DOMObject {
    4040    public:
    4141        PluginBase(ExecState *exec);
     
    142142KJS_IMPLEMENT_PROTOFUNC(NavigatorFunc)
    143143
    144 Navigator::Navigator(ExecState *exec, Frame *p)
    145   : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype()), m_frame(p) { }
     144Navigator::Navigator(ExecState *exec, Frame *f)
     145    : m_frame(f)
     146{
     147    setPrototype(exec->lexicalInterpreter()->builtinObjectPrototype());
     148}
    146149
    147150bool Navigator::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
     
    225228
    226229PluginBase::PluginBase(ExecState *exec)
    227   : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype() )
    228 {
     230{
     231    setPrototype(exec->lexicalInterpreter()->builtinObjectPrototype());
     232
    229233    cachePluginDataIfNecessary();
    230234    m_plugInCacheRefCount++;
  • trunk/WebCore/khtml/ecma/kjs_navigator.h

    r13014 r13568  
    2222#define KJS_Navigator_H
    2323
     24#include "kjs_binding.h"
    2425#include <kjs/object.h>
    2526
     
    3031namespace KJS {
    3132
    32   class Navigator : public JSObject {
     33  class Navigator : public DOMObject {
    3334  public:
    3435    Navigator(ExecState *exec, WebCore::Frame *p);
  • trunk/WebCore/khtml/ecma/kjs_proxy.cpp

    r13527 r13568  
    126126
    127127// Implementation of the debug() function
    128 class TestFunctionImp : public JSObject {
     128class TestFunctionImp : public DOMObject {
    129129public:
    130   TestFunctionImp() : JSObject() {}
    131130  virtual bool implementsCall() const { return true; }
    132131  virtual JSValue *callAsFunction(ExecState *exec, JSObject *thisObj, const List &args);
  • trunk/WebCore/khtml/ecma/kjs_window.cpp

    r13532 r13568  
    9696////////////////////// History Object ////////////////////////
    9797
    98   class History : public JSObject {
     98  class History : public DOMObject {
    9999    friend class HistoryFunc;
    100100  public:
    101     History(ExecState *exec, Frame *p)
    102       : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype()), m_frame(p) { }
     101    History(ExecState *exec, Frame *f)
     102      : m_frame(f)
     103    {
     104      setPrototype(exec->lexicalInterpreter()->builtinObjectPrototype());
     105    }
    103106    virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
    104107    JSValue *getValueProperty(ExecState *exec, int token) const;
     
    112115  };
    113116
    114   class FrameArray : public JSObject {
     117
     118  class FrameArray : public DOMObject {
    115119  public:
    116     FrameArray(ExecState *exec, Frame *p)
    117       : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype()), m_frame(p) { }
     120    FrameArray(ExecState *exec, Frame *f)
     121      : m_frame(f)
     122    {
     123      setPrototype(exec->lexicalInterpreter()->builtinObjectPrototype());
     124    }
    118125    virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
    119126    JSValue *getValueProperty(ExecState *exec, int token);
     
    157164// We set the object prototype so that toString is implemented
    158165Screen::Screen(ExecState* exec, Frame* f)
    159   : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype()), m_frame(f)
    160 {
     166  : m_frame(f)
     167{
     168     setPrototype(exec->lexicalInterpreter()->builtinObjectPrototype());
    161169}
    162170
     
    307315KJS_IMPLEMENT_PROTOFUNC(WindowFunc)
    308316
    309 Window::Window(Frame *p)
    310   : JSObject(/*no proto*/)
    311   , m_frame(p)
     317Window::Window(Frame *f)
     318  : m_frame(f)
    312319  , screen(0)
    313320  , history(0)
     
    23782385@end
    23792386*/
    2380 BarInfo::BarInfo(ExecState *exec, Frame *p, Type barType)
    2381   : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype())
    2382   , m_frame(p)
     2387BarInfo::BarInfo(ExecState *exec, Frame *f, Type barType)
     2388  : m_frame(f)
    23832389  , m_type(barType)
    23842390{
     2391  setPrototype(exec->lexicalInterpreter()->builtinObjectPrototype());
    23852392}
    23862393
  • trunk/WebCore/khtml/ecma/kjs_window.h

    r13393 r13568  
    6868    class DOMWindowTimer;
    6969
    70   class Screen : public JSObject {
     70  class Screen : public DOMObject {
    7171  public:
    7272    enum { Height, Width, ColorDepth, PixelDepth, AvailLeft, AvailTop, AvailHeight, AvailWidth };
     
    8181  };
    8282
    83   class Window : public JSObject {
     83  class Window : public DOMObject {
    8484    friend class Location;
    8585    friend class WindowFunc;
     
    219219    };
    220220
    221   class Location : public JSObject {
     221  class Location : public DOMObject {
    222222  public:
    223223    virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
     
    237237  };
    238238
    239   class Selection : public JSObject {
     239  class Selection : public DOMObject {
    240240  public:
    241241    virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
     
    255255  };
    256256
    257   class BarInfo : public JSObject {
     257  class BarInfo : public DOMObject {
    258258  public:
    259259    virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
Note: See TracChangeset for help on using the changeset viewer.