Bug 602139: Add js_UpdateWatchpointsForShape, to correctly update watchpoints after shape changes. r=jorendorff
authorJim Blandy <jimb@mozilla.com>
Tue, 09 Nov 2010 15:04:12 -0800
changeset 57752 4e9b098ccf87ce74adc3393473ee935df8a85b45
parent 57751 94543969fd6d48d57019aa3ee5a28a90048497bc
child 57753 c146eeb9fecc942e2ed9a8513f83e37d4493fc91
push id17032
push userrsayre@mozilla.com
push dateWed, 17 Nov 2010 21:55:39 +0000
treeherdermozilla-central@78a42f77bb90 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs602139
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 602139: Add js_UpdateWatchpointsForShape, to correctly update watchpoints after shape changes. r=jorendorff Many of the watchpoint bugs have to do with wp->setter and wp->shape getting out of sync. The new function js_UpdateWatchpointsForShape takes care of bringing all relevant watchpoints fully up to date; it is called from the places in jsscope.cpp where property changes take place, and is also used by the watchpoint creation code. DropWatchPointAndUnlock becomes simpler; js_FindWatchPoint, js_WrapWatchedSetter and js_watch_set_wrapper become static to jsdbgapi.cpp.
js/src/jsdbgapi.cpp
js/src/jsdbgapi.h
js/src/jsdbgapiinlines.h
js/src/jsscope.cpp
--- a/js/src/jsdbgapi.cpp
+++ b/js/src/jsdbgapi.cpp
@@ -60,16 +60,17 @@
 #include "jsparse.h"
 #include "jsscope.h"
 #include "jsscript.h"
 #include "jsstaticcheck.h"
 #include "jsstr.h"
 #include "jswrapper.h"
 
 #include "jsatominlines.h"
+#include "jsdbgapiinlines.h"
 #include "jsinterpinlines.h"
 #include "jsobjinlines.h"
 #include "jsscopeinlines.h"
 
 #include "jsautooplen.h"
 
 #include "methodjit/MethodJIT.h"
 #include "methodjit/Retcon.h"
@@ -540,51 +541,34 @@ DropWatchPointAndUnlock(JSContext *cx, J
     JSRuntime *rt = cx->runtime;
 
     wp->flags &= ~flag;
     if (wp->flags != 0) {
         DBG_UNLOCK(rt);
         return ok;
     }
 
-    /*
-     * Remove wp from the list, then if there are no other watchpoints for
-     * wp->shape in any scope, restore wp->shape->setter from wp.
-     */
+    /* Remove wp from the list, then restore wp->shape->setter from wp. */
     ++rt->debuggerMutations;
     JS_REMOVE_LINK(&wp->links);
-
-    const Shape *shape = wp->shape;
-    PropertyOp setter = NULL;
-
-    for (JSWatchPoint *wp2 = (JSWatchPoint *)rt->watchPointList.next;
-         &wp2->links != &rt->watchPointList;
-         wp2 = (JSWatchPoint *)wp2->links.next) {
-        if (wp2->shape == shape) {
-            setter = wp->setter;
-            break;
-        }
-    }
     DBG_UNLOCK(rt);
 
-    if (!setter) {
-        /*
-         * If the property wasn't found on wp->object, or it isn't still being
-         * watched, then someone else must have deleted or unwatched it, and we
-         * don't need to change the property attributes.
-         */
-        const Shape *wprop = wp->object->nativeLookup(shape->id);
-        if (wprop &&
-            wprop->hasSetterValue() == shape->hasSetterValue() &&
-            IsWatchedProperty(cx, wprop)) {
-            shape = wp->object->changeProperty(cx, wprop, 0, wprop->attributes(),
-                                               wprop->getter(), wp->setter);
-            if (!shape)
-                ok = false;
-        }
+    /*
+     * If the property isn't found on wp->object, then someone else must have deleted it,
+     * and we don't need to change the property attributes.
+     */
+    const Shape *shape = wp->shape;
+    const Shape *wprop = wp->object->nativeLookup(shape->id);
+    if (wprop &&
+        wprop->hasSetterValue() == shape->hasSetterValue() &&
+        IsWatchedProperty(cx, wprop)) {
+        shape = wp->object->changeProperty(cx, wprop, 0, wprop->attributes(),
+                                           wprop->getter(), wp->setter);
+        if (!shape)
+            ok = false;
     }
 
     cx->free(wp);
     return ok;
 }
 
 /*
  * NB: js_TraceWatchPoints does not acquire cx->runtime->debuggerLock, since
@@ -635,43 +619,41 @@ js_SweepWatchPoints(JSContext *cx)
         }
     }
     DBG_UNLOCK(rt);
 }
 
 
 
 /*
- * NB: FindWatchPoint must be called with rt->debuggerLock acquired.
+ * NB: LockedFindWatchPoint must be called with rt->debuggerLock acquired.
  */
 static JSWatchPoint *
-FindWatchPoint(JSRuntime *rt, JSObject *obj, jsid id)
+LockedFindWatchPoint(JSRuntime *rt, JSObject *obj, jsid id)
 {
     JSWatchPoint *wp;
 
     for (wp = (JSWatchPoint *)rt->watchPointList.next;
          &wp->links != &rt->watchPointList;
          wp = (JSWatchPoint *)wp->links.next) {
         if (wp->object == obj && wp->shape->id == id)
             return wp;
     }
     return NULL;
 }
 
-const Shape *
-js_FindWatchPoint(JSRuntime *rt, JSObject *obj, jsid id)
+static JSWatchPoint *
+FindWatchPoint(JSRuntime *rt, JSObject *obj, jsid id)
 {
     JSWatchPoint *wp;
-    const Shape *shape;
 
     DBG_LOCK(rt);
-    wp = FindWatchPoint(rt, obj, id);
-    shape = wp ? wp->shape : NULL;
+    wp = LockedFindWatchPoint(rt, obj, id);
     DBG_UNLOCK(rt);
-    return shape;
+    return wp;
 }
 
 JSBool
 js_watch_set(JSContext *cx, JSObject *obj, jsid id, Value *vp)
 {
     JSRuntime *rt = cx->runtime;
     DBG_LOCK(rt);
     for (JSWatchPoint *wp = (JSWatchPoint *)rt->watchPointList.next;
@@ -711,17 +693,17 @@ js_watch_set(JSContext *cx, JSObject *ob
             DBG_LOCK(rt);
             return DropWatchPointAndUnlock(cx, wp, JSWP_HELD) && ok;
         }
     }
     DBG_UNLOCK(rt);
     return JS_TRUE;
 }
 
-JSBool
+static JSBool
 js_watch_set_wrapper(JSContext *cx, uintN argc, Value *vp)
 {
     JSObject *obj = ComputeThisFromVp(cx, vp);
     if (!obj)
         return false;
 
     JSObject &funobj = JS_CALLEE(cx, vp).toObject();
     JSFunction *wrapper = funobj.getFunctionPrivate();
@@ -745,18 +727,18 @@ IsWatchedProperty(JSContext *cx, const S
     return shape->setterOp() == js_watch_set;
 }
 
 /*
  * Return an appropriate setter to substitute for |setter| on a property
  * with attributes |attrs|, to implement a watchpoint on the property named
  * |id|.
  */
-PropertyOp
-js_WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, PropertyOp setter)
+static PropertyOp
+WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, PropertyOp setter)
 {
     JSAtom *atom;
     JSFunction *wrapper;
 
     /* Wrap a JSPropertyOp setter simply by returning our own JSPropertyOp. */
     if (!(attrs & JSPROP_SETTER))
         return &js_watch_set;   /* & to silence schoolmarmish MSVC */
 
@@ -776,16 +758,98 @@ js_WrapWatchedSetter(JSContext *cx, jsid
 
     wrapper = js_NewFunction(cx, NULL, js_watch_set_wrapper, 1, 0,
                              setter ? CastAsObject(setter)->getParent() : NULL, atom);
     if (!wrapper)
         return NULL;
     return CastAsPropertyOp(FUN_OBJECT(wrapper));
 }
 
+static bool
+UpdateWatchpointShape(JSContext *cx, JSWatchPoint *wp, const js::Shape *newShape)
+{
+    JS_ASSERT_IF(wp->shape, wp->shape->id == newShape->id);
+    JS_ASSERT(!IsWatchedProperty(cx, newShape));
+
+    /* Create a watching setter we can substitute for the new shape's setter. */
+    js::PropertyOp watchingSetter = WrapWatchedSetter(cx, newShape->id, newShape->attributes(),
+                                                      newShape->setter());
+    if (!watchingSetter)
+        return false;
+
+    /*
+     * Save the shape's setter; we don't know whether js_ChangeNativePropertyAttrs will
+     * return a new shape, or mutate this one.
+     */
+    js::PropertyOp originalSetter = newShape->setter();
+
+    /*
+     * Drop the watching setter into the object, in place of newShape. Note that a single
+     * watchpoint-wrapped shape may correspond to more than one non-watchpoint shape: we
+     * wrap all (JSPropertyOp, not JSObject *) setters with js_watch_set, so shapes that
+     * differ only in their setter may all get wrapped to the same shape.
+     */
+    const js::Shape *watchingShape = 
+        js_ChangeNativePropertyAttrs(cx, wp->object, newShape, 0, newShape->attributes(),
+                                     newShape->getter(), watchingSetter);
+    if (!watchingShape)
+        return false;
+
+    /* Update the watchpoint with the new shape and its original setter. */
+    wp->setter = originalSetter;
+    wp->shape = watchingShape;
+
+    return true;
+}
+
+bool
+js_SlowPathUpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const js::Shape *newShape)
+{
+    /*
+     * The watchpoint code uses the normal property-modification functions to install its
+     * own watchpoint-aware shapes. Those functions report those changes back to the
+     * watchpoint code, just as they do user-level changes. So if this change is
+     * installing a watchpoint-aware shape, it's something we asked for ourselves, and can
+     * proceed without interference.
+     */
+    if (IsWatchedProperty(cx, newShape))
+        return true;
+
+    JSWatchPoint *wp = FindWatchPoint(cx->runtime, obj, newShape->id);
+    if (!wp)
+        return true;
+
+    return UpdateWatchpointShape(cx, wp, newShape);
+}
+
+/*
+ * Return the underlying setter for |shape| on |obj|, seeing through any
+ * watchpoint-wrapping. Note that we need |obj| to disambiguate, since a single
+ * watchpoint-wrapped shape may correspond to more than one non-watchpoint shape; see the
+ * comments in UpdateWatchpointShape.
+ */
+static PropertyOp
+UnwrapSetter(JSContext *cx, JSObject *obj, const Shape *shape)
+{
+    /* If it's not a watched property, its setter is not wrapped. */
+    if (!IsWatchedProperty(cx, shape))
+        return shape->setter();
+
+    /* Look up the watchpoint, from which we can retrieve the underlying setter. */
+    JSWatchPoint *wp = FindWatchPoint(cx->runtime, obj, shape->id);
+
+    /* 
+     * Since we know |shape| is watched, we *must* find a watchpoint: we should never
+     * leave wrapped setters lying around in shapes after removing a watchpoint.
+     */
+    JS_ASSERT(wp);
+
+    return wp->setter;
+}
+
 JS_PUBLIC_API(JSBool)
 JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsid id,
                  JSWatchPointHandler handler, JSObject *closure)
 {
     JSObject *origobj;
     Value v;
     uintN attrs;
     jsid propid;
@@ -813,26 +877,26 @@ JS_SetWatchPoint(JSContext *cx, JSObject
         return JS_FALSE;
 
     if (!obj->isNative()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_WATCH,
                              obj->getClass()->name);
         return JS_FALSE;
     }
 
+    JSObject *pobj;
     JSProperty *prop;
-    JSObject *pobj;
     if (!js_LookupProperty(cx, obj, propid, &pobj, &prop))
         return JS_FALSE;
     const Shape *shape = (Shape *) prop;
     JSRuntime *rt = cx->runtime;
     if (!shape) {
         /* Check for a deleted symbol watchpoint, which holds its property. */
-        shape = js_FindWatchPoint(rt, obj, propid);
-        if (!shape) {
+        JSWatchPoint *wp = FindWatchPoint(rt, obj, propid);
+        if (!wp) {
             /* Make a new property in obj so we can watch for the first set. */
             if (!js_DefineNativeProperty(cx, obj, propid, UndefinedValue(), NULL, NULL,
                                          JSPROP_ENUMERATE, 0, 0, &prop)) {
                 return JS_FALSE;
             }
             shape = (Shape *) prop;
         }
     } else if (pobj != obj) {
@@ -842,17 +906,17 @@ JS_SetWatchPoint(JSContext *cx, JSObject
         uintN attrs, flags;
         intN shortid;
 
         if (pobj->isNative()) {
             valroot.set(pobj->containsSlot(shape->slot)
                         ? pobj->nativeGetSlot(shape->slot)
                         : UndefinedValue());
             getter = shape->getter();
-            setter = shape->setter();
+            setter = UnwrapSetter(cx, pobj, shape);
             attrs = shape->attributes();
             flags = shape->getFlags();
             shortid = shape->shortid;
         } else {
             if (!pobj->getProperty(cx, propid, valroot.addr()) ||
                 !pobj->getAttributes(cx, propid, &attrs)) {
                 return JS_FALSE;
             }
@@ -870,51 +934,44 @@ JS_SetWatchPoint(JSContext *cx, JSObject
         shape = (Shape *) prop;
     }
 
     /*
      * At this point, prop/shape exists in obj, obj is locked, and we must
      * unlock the object before returning.
      */
     DBG_LOCK(rt);
-    JSWatchPoint *wp = FindWatchPoint(rt, obj, propid);
+    JSWatchPoint *wp = LockedFindWatchPoint(rt, obj, propid);
     if (!wp) {
         DBG_UNLOCK(rt);
-        PropertyOp watcher = js_WrapWatchedSetter(cx, propid, shape->attributes(), shape->setter());
-        if (!watcher)
-            return JS_FALSE;
-
         wp = (JSWatchPoint *) cx->malloc(sizeof *wp);
         if (!wp)
             return JS_FALSE;
         wp->handler = NULL;
         wp->closure = NULL;
         wp->object = obj;
-        wp->setter = shape->setter();
+        wp->shape = NULL;
         wp->flags = JSWP_LIVE;
 
         /* XXXbe nest in obj lock here */
-        shape = js_ChangeNativePropertyAttrs(cx, obj, shape, 0, shape->attributes(),
-                                             shape->getter(), watcher);
-        if (!shape) {
+        if (!UpdateWatchpointShape(cx, wp, shape)) {
             /* Self-link so DropWatchPointAndUnlock can JS_REMOVE_LINK it. */
             JS_INIT_CLIST(&wp->links);
             DBG_LOCK(rt);
             DropWatchPointAndUnlock(cx, wp, JSWP_LIVE);
             return JS_FALSE;
         }
-        wp->shape = shape;
 
         /*
          * Now that wp is fully initialized, append it to rt's wp list.
          * Because obj is locked we know that no other thread could have added
          * a watchpoint for (obj, propid).
          */
         DBG_LOCK(rt);
-        JS_ASSERT(!FindWatchPoint(rt, obj, propid));
+        JS_ASSERT(!LockedFindWatchPoint(rt, obj, propid));
         JS_APPEND_LINK(&wp->links, &rt->watchPointList);
         ++rt->debuggerMutations;
     }
     wp->handler = handler;
     wp->closure = reinterpret_cast<JSObject*>(closure);
     DBG_UNLOCK(rt);
     return JS_TRUE;
 }
--- a/js/src/jsdbgapi.h
+++ b/js/src/jsdbgapi.h
@@ -137,28 +137,19 @@ JS_ClearAllWatchPoints(JSContext *cx);
 extern void
 js_TraceWatchPoints(JSTracer *trc, JSObject *obj);
 
 extern void
 js_SweepWatchPoints(JSContext *cx);
 
 #ifdef __cplusplus
 
-extern const js::Shape *
-js_FindWatchPoint(JSRuntime *rt, JSObject *obj, jsid id);
-
 extern JSBool
 js_watch_set(JSContext *cx, JSObject *obj, jsid id, js::Value *vp);
 
-extern JSBool
-js_watch_set_wrapper(JSContext *cx, uintN argc, js::Value *vp);
-
-extern js::PropertyOp
-js_WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, js::PropertyOp setter);
-
 #endif
 
 #endif /* JS_HAS_OBJ_WATCHPOINT */
 
 /************************************************************************/
 
 extern JS_PUBLIC_API(uintN)
 JS_PCToLineNumber(JSContext *cx, JSScript *script, jsbytecode *pc);
new file mode 100644
--- /dev/null
+++ b/js/src/jsdbgapiinlines.h
@@ -0,0 +1,68 @@
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=4 sw=4 et tw=78:
+ *
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is SpiderMonkey code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Jim Blandy <jimb@mozilla.com> (original author)
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of the GNU General Public License Version 2 or later (the "GPL"),
+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#ifndef jsdbgapiinlines_h___
+#define jsdbgapiinlines_h___
+
+#include "jsdbgapi.h"
+#include "jscntxt.h"
+
+#if defined(JS_HAS_OBJ_WATCHPOINT) && defined(__cplusplus)
+
+extern bool
+js_SlowPathUpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const js::Shape *newShape);
+
+/*
+ * Update any watchpoints on |obj| on |new_shape->id| to use |new_shape|. Property-manipulating
+ * functions must call this any time it takes on a new shape to represent a potentially
+ * watched property, or when it mutates a shape's attributes/setter/getter.
+ */
+static inline bool
+js_UpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const js::Shape *newShape)
+{
+    if (JS_CLIST_IS_EMPTY(&cx->runtime->watchPointList))
+        return true;
+
+    return js_SlowPathUpdateWatchpointsForShape(cx, obj, newShape);
+}
+
+#endif /* JS_HAS_OBJ_WATCHPOINT && __cplusplus */
+
+#endif /* jsdbgapiinlines_h__ */
--- a/js/src/jsscope.cpp
+++ b/js/src/jsscope.cpp
@@ -58,16 +58,17 @@
 #include "jsfun.h"      /* for JS_ARGS_LENGTH_MAX */
 #include "jslock.h"
 #include "jsnum.h"
 #include "jsobj.h"
 #include "jsscope.h"
 #include "jsstr.h"
 #include "jstracer.h"
 
+#include "jsdbgapiinlines.h"
 #include "jsobjinlines.h"
 #include "jsscopeinlines.h"
 
 using namespace js;
 using namespace js::gc;
 
 uint32
 js_GenerateShape(JSContext *cx, bool gcLocked)
@@ -609,29 +610,16 @@ NormalizeGetterAndSetter(JSContext *cx, 
         JS_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER)));
     } else {
         if (getter == PropertyStub) {
             JS_ASSERT(!(attrs & JSPROP_GETTER));
             getter = NULL;
         }
     }
 
-    /*
-     * Check for a watchpoint on a deleted property; if one exists, change
-     * setter to js_watch_set or js_watch_set_wrapper.
-     * XXXbe this could get expensive with lots of watchpoints...
-     */
-    if (!JS_CLIST_IS_EMPTY(&cx->runtime->watchPointList) &&
-        js_FindWatchPoint(cx->runtime, obj, id)) {
-        setter = js_WrapWatchedSetter(cx, id, attrs, setter);
-        if (!setter) {
-            METER(wrapWatchFails);
-            return false;
-        }
-    }
     return true;
 }
 
 #ifdef DEBUG
 # define CHECK_SHAPE_CONSISTENCY(obj) obj->checkShapeConsistency()
 
 void
 JSObject::checkShapeConsistency()
@@ -726,17 +714,28 @@ JSObject::addProperty(JSContext *cx, jsi
         return NULL;
     }
 
     NormalizeGetterAndSetter(cx, this, id, attrs, flags, getter, setter);
 
     /* Search for id with adding = true in order to claim its entry. */
     Shape **spp = nativeSearch(id, true);
     JS_ASSERT(!SHAPE_FETCH(spp));
-    return addPropertyInternal(cx, id, getter, setter, slot, attrs, flags, shortid, spp);
+    const Shape *shape = addPropertyInternal(cx, id, getter, setter, slot, attrs, 
+                                             flags, shortid, spp);
+    if (!shape)
+        return NULL;
+
+    /* Update any watchpoints referring to this property. */
+    if (!js_UpdateWatchpointsForShape(cx, this, shape)) {
+        METER(wrapWatchFails);
+        return NULL;
+    }
+
+    return shape;
 }
 
 const Shape *
 JSObject::addPropertyInternal(JSContext *cx, jsid id,
                               PropertyOp getter, PropertyOp setter,
                               uint32 slot, uintN attrs,
                               uintN flags, intN shortid,
                               Shape **spp)
@@ -845,17 +844,23 @@ JSObject::putProperty(JSContext *cx, jsi
          * You can't add properties to a non-extensible object, but you can change
          * attributes of properties in such objects.
          */
         if (!isExtensible()) {
             reportNotExtensible(cx);
             return NULL;
         }
 
-        return addPropertyInternal(cx, id, getter, setter, slot, attrs, flags, shortid, spp);
+        const Shape *new_shape =
+            addPropertyInternal(cx, id, getter, setter, slot, attrs, flags, shortid, spp);
+        if (!js_UpdateWatchpointsForShape(cx, this, new_shape)) {
+            METER(wrapWatchFails);
+            return NULL;
+        }
+        return new_shape;
     }
 
     /* Property exists: search must have returned a valid *spp. */
     JS_ASSERT(!SHAPE_IS_REMOVED(*spp));
 
     /*
      * If the caller wants to allocate a slot, but doesn't care which slot,
      * copy the existing shape's slot into slot so we can match shape, if all
@@ -877,17 +882,17 @@ JSObject::putProperty(JSContext *cx, jsi
 
     /*
      * Overwriting a non-last property requires switching to dictionary mode.
      * The shape tree is shared immutable, and we can't removeProperty and then
      * addPropertyInternal because a failure under add would lose data.
      */
     if (shape != lastProp && !inDictionaryMode()) {
         if (!toDictionaryMode(cx))
-            return false;
+            return NULL;
         spp = nativeSearch(shape->id);
         shape = SHAPE_FETCH(spp);
     }
 
     /*
      * Now that we have passed the lastProp->frozen() check at the top of this
      * method, and the non-last-property conditioning just above, we are ready
      * to overwrite.
@@ -982,16 +987,22 @@ JSObject::putProperty(JSContext *cx, jsi
         else
             getSlotRef(oldSlot).setUndefined();
 #endif
         JS_ATOMIC_INCREMENT(&cx->runtime->propertyRemovals);
     }
 
     CHECK_SHAPE_CONSISTENCY(this);
     METER(puts);
+
+    if (!js_UpdateWatchpointsForShape(cx, this, shape)) {
+        METER(wrapWatchFails);
+        return NULL;
+    }
+
     return shape;
 }
 
 const Shape *
 JSObject::changeProperty(JSContext *cx, const Shape *shape, uintN attrs, uintN mask,
                          PropertyOp getter, PropertyOp setter)
 {
     JS_ASSERT_IF(inDictionaryMode(), !lastProp->frozen());
@@ -1037,16 +1048,21 @@ JSObject::changeProperty(JSContext *cx, 
 
                 /* Hand the table off from tableShape to newShape. */
                 tableShape->setTable(NULL);
                 newShape->setTable(table);
             }
 
             updateFlags(newShape);
             updateShape(cx);
+
+            if (!js_UpdateWatchpointsForShape(cx, this, newShape)) {
+                METER(wrapWatchFails);
+                return NULL;
+            }
         }
     } else if (shape == lastProp) {
         newShape = getChildProperty(cx, shape->parent, child);
 #ifdef DEBUG
         if (newShape) {
             JS_ASSERT(newShape == lastProp);
             if (newShape->table) {
                 Shape **spp = nativeSearch(shape->id);