Bug 626118 - Fix RegExp ref-counting leak; record already-incremented-ness in type (r=cdleary)

This commit is contained in:
Luke Wagner 2011-01-17 14:58:53 -08:00
parent fbd83bbc4f
commit 68eefec80e
5 changed files with 129 additions and 76 deletions

View File

@ -2788,35 +2788,93 @@ class AutoLocalNameArray {
};
template <class RefCountable>
class AutoRefCount {
JSContext * const cx;
RefCountable *obj;
class AlreadyIncRefed
{
typedef RefCountable *****ConvertibleToBool;
RefCountable *obj;
public:
explicit AutoRefCount(JSContext *cx) : cx(cx), obj(NULL) {}
explicit AlreadyIncRefed(RefCountable *obj) : obj(obj) {}
AutoRefCount(JSContext *cx, RefCountable *obj) : cx(cx), obj(NULL) {
reset(obj);
bool null() const { return obj == NULL; }
operator ConvertibleToBool() const { return (ConvertibleToBool)obj; }
RefCountable *operator->() const { JS_ASSERT(!null()); return obj; }
RefCountable &operator*() const { JS_ASSERT(!null()); return *obj; }
RefCountable *get() const { return obj; }
};
template <class RefCountable>
class NeedsIncRef
{
typedef RefCountable *****ConvertibleToBool;
RefCountable *obj;
public:
explicit NeedsIncRef(RefCountable *obj) : obj(obj) {}
bool null() const { return obj == NULL; }
operator ConvertibleToBool() const { return (ConvertibleToBool)obj; }
RefCountable *operator->() const { JS_ASSERT(!null()); return obj; }
RefCountable &operator*() const { JS_ASSERT(!null()); return *obj; }
RefCountable *get() const { return obj; }
};
template <class RefCountable>
class AutoRefCount
{
typedef RefCountable *****ConvertibleToBool;
JSContext *const cx;
RefCountable *obj;
AutoRefCount(const AutoRefCount &);
void operator=(const AutoRefCount &);
public:
explicit AutoRefCount(JSContext *cx)
: cx(cx), obj(NULL)
{}
AutoRefCount(JSContext *cx, NeedsIncRef<RefCountable> aobj)
: cx(cx), obj(aobj.get())
{
if (obj)
obj->incref(cx);
}
AutoRefCount(JSContext *cx, AlreadyIncRefed<RefCountable> aobj)
: cx(cx), obj(aobj.get())
{}
~AutoRefCount() {
if (obj)
obj->decref(cx);
}
void reset(RefCountable *aobj) {
void reset(NeedsIncRef<RefCountable> aobj) {
if (obj)
obj->decref(cx);
obj = aobj;
obj = aobj.get();
if (obj)
obj->incref(cx);
}
RefCountable *get() {
return obj;
void reset(AlreadyIncRefed<RefCountable> aobj) {
if (obj)
obj->decref(cx);
obj = aobj.get();
}
bool null() const { return obj == NULL; }
operator ConvertibleToBool() const { return (ConvertibleToBool)obj; }
RefCountable *operator->() const { JS_ASSERT(!null()); return obj; }
RefCountable &operator*() const { JS_ASSERT(!null()); return *obj; }
RefCountable *get() const { return obj; }
};
} /* namespace js */

View File

@ -4706,10 +4706,7 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, const Value &valu
if (defineHow & JSDNP_CACHE_RESULT) {
JS_ASSERT_NOT_ON_TRACE(cx);
if (added) {
#ifdef JS_TRACER
PropertyCacheEntry *entry =
#endif
JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, 0, obj, shape, true);
JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, 0, obj, shape, true);
TRACE_1(AddProperty, obj);
}
}

View File

@ -118,14 +118,14 @@ Class js::regexp_statics_class = {
* Note that the refcount of |newRegExp| is unchanged.
*/
static void
SwapObjectRegExp(JSContext *cx, JSObject *obj, RegExp &newRegExp)
SwapObjectRegExp(JSContext *cx, JSObject *obj, AlreadyIncRefed<RegExp> newRegExp)
{
RegExp *oldRegExp = RegExp::extractFrom(obj);
#ifdef DEBUG
assertSameCompartment(cx, obj, newRegExp.compartment);
assertSameCompartment(cx, obj, newRegExp->compartment);
#endif
obj->setPrivate(&newRegExp);
obj->setPrivate(newRegExp.get());
obj->zeroRegExpLastIndex();
if (oldRegExp)
oldRegExp->decref(cx);
@ -158,9 +158,10 @@ js_CloneRegExpObject(JSContext *cx, JSObject *obj, JSObject *proto)
* This regex is lacking flags from the statics, so we must recompile with the new
* flags instead of increffing.
*/
re = RegExp::create(cx, re->getSource(), origFlags | staticsFlags);
if (!re)
AlreadyIncRefed<RegExp> clone = RegExp::create(cx, re->getSource(), origFlags | staticsFlags);
if (!clone)
return NULL;
re = clone.get();
} else {
re->incref(cx);
}
@ -282,14 +283,14 @@ RegExp::parseFlags(JSContext *cx, JSString *flagStr, uint32 &flagsOut)
return true;
}
RegExp *
AlreadyIncRefed<RegExp>
RegExp::createFlagged(JSContext *cx, JSString *str, JSString *opt)
{
if (!opt)
return create(cx, str, 0);
uint32 flags = 0;
if (!parseFlags(cx, opt, flags))
return false;
return AlreadyIncRefed<RegExp>(NULL);
return create(cx, str, flags);
}
@ -504,10 +505,10 @@ js_XDRRegExpObject(JSXDRState *xdr, JSObject **objp)
return false;
obj->clearParent();
obj->clearProto();
RegExp *re = RegExp::create(xdr->cx, source, flagsword);
AlreadyIncRefed<RegExp> re = RegExp::create(xdr->cx, source, flagsword);
if (!re)
return false;
obj->setPrivate(re);
obj->setPrivate(re.get());
obj->zeroRegExpLastIndex();
*objp = obj;
}
@ -666,10 +667,10 @@ static bool
regexp_compile_sub_tail(JSContext *cx, JSObject *obj, Value *rval, JSString *str, uint32 flags = 0)
{
flags |= cx->regExpStatics()->getFlags();
RegExp *re = RegExp::create(cx, str, flags);
AlreadyIncRefed<RegExp> re = RegExp::create(cx, str, flags);
if (!re)
return false;
SwapObjectRegExp(cx, obj, *re);
SwapObjectRegExp(cx, obj, re);
*rval = ObjectValue(*obj);
return true;
}
@ -697,16 +698,13 @@ regexp_compile_sub(JSContext *cx, JSObject *obj, uintN argc, Value *argv, Value
JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_NEWREGEXP_FLAGGED);
return false;
}
RegExp *clone;
{
RegExp *re = RegExp::extractFrom(&sourceObj);
if (!re)
return false;
clone = RegExp::clone(cx, *re);
}
RegExp *re = RegExp::extractFrom(&sourceObj);
if (!re)
return false;
AlreadyIncRefed<RegExp> clone = RegExp::clone(cx, *re);
if (!clone)
return false;
SwapObjectRegExp(cx, obj, *clone);
SwapObjectRegExp(cx, obj, clone);
*rval = ObjectValue(*obj);
return true;
}
@ -755,7 +753,7 @@ regexp_exec_sub(JSContext *cx, JSObject *obj, uintN argc, Value *argv, JSBool te
* Code execution under this call could swap out the guts of |obj|, so we
* have to take a defensive refcount here.
*/
AutoRefCount<RegExp> arc(cx, re);
AutoRefCount<RegExp> arc(cx, NeedsIncRef<RegExp>(re));
jsdouble lastIndex;
if (re->global() || re->sticky()) {
@ -876,10 +874,10 @@ regexp_construct(JSContext *cx, uintN argc, Value *vp)
static bool
InitRegExpClassCompile(JSContext *cx, JSObject *obj)
{
RegExp *re = RegExp::create(cx, cx->runtime->emptyString, 0);
AlreadyIncRefed<RegExp> re = RegExp::create(cx, cx->runtime->emptyString, 0);
if (!re)
return false;
SwapObjectRegExp(cx, obj, *re);
SwapObjectRegExp(cx, obj, re);
return true;
}

View File

@ -112,6 +112,16 @@ class RegExp
#endif
{ }
~RegExp() {
#if !ENABLE_YARR_JIT
if (compiled)
jsRegExpFree(compiled);
#endif
}
/* Constructor/destructor are hidden; called by cx->create/destroy. */
friend struct ::JSContext;
bool compileHelper(JSContext *cx, JSLinearString &pattern);
bool compile(JSContext *cx);
static const uint32 allFlags = JSREG_FOLD | JSREG_GLOB | JSREG_MULTILINE | JSREG_STICKY;
@ -124,13 +134,6 @@ class RegExp
size_t *lastIndex, bool test, Value *rval);
public:
~RegExp() {
#if !ENABLE_YARR_JIT
if (compiled)
jsRegExpFree(compiled);
#endif
}
static inline bool isMetaChar(jschar c);
static inline bool hasMetaChars(const jschar *chars, size_t length);
@ -160,10 +163,10 @@ class RegExp
/* Factories */
static RegExp *create(JSContext *cx, JSString *source, uint32 flags);
static AlreadyIncRefed<RegExp> create(JSContext *cx, JSString *source, uint32 flags);
/* Would overload |create|, but |0| resolves ambiguously against pointer and uint. */
static RegExp *createFlagged(JSContext *cx, JSString *source, JSString *flags);
static AlreadyIncRefed<RegExp> createFlagged(JSContext *cx, JSString *source, JSString *flags);
/*
* Create an object with new regular expression internals.
@ -176,7 +179,7 @@ class RegExp
static JSObject *createObjectNoStatics(JSContext *cx, const jschar *chars, size_t length,
uint32 flags);
static RegExp *extractFrom(JSObject *obj);
static RegExp *clone(JSContext *cx, const RegExp &other);
static AlreadyIncRefed<RegExp> clone(JSContext *cx, const RegExp &other);
/* Mutators */
@ -394,22 +397,21 @@ RegExp::executeInternal(JSContext *cx, RegExpStatics *res, JSString *inputstr,
return true;
}
inline RegExp *
inline AlreadyIncRefed<RegExp>
RegExp::create(JSContext *cx, JSString *source, uint32 flags)
{
typedef AlreadyIncRefed<RegExp> RetType;
JSLinearString *flatSource = source->ensureLinear(cx);
if (!flatSource)
return NULL;
RegExp *self;
void *mem = cx->malloc(sizeof(*self));
if (!mem)
return NULL;
self = new (mem) RegExp(flatSource, flags, cx->compartment);
return RetType(NULL);
RegExp *self = cx->create<RegExp>(flatSource, flags, cx->compartment);
if (!self)
return RetType(NULL);
if (!self->compile(cx)) {
cx->destroy<RegExp>(self);
return NULL;
return RetType(NULL);
}
return self;
return RetType(self);
}
inline JSObject *
@ -427,7 +429,7 @@ RegExp::createObjectNoStatics(JSContext *cx, const jschar *chars, size_t length,
JSString *str = js_NewStringCopyN(cx, chars, length);
if (!str)
return NULL;
RegExp *re = RegExp::create(cx, str, flags);
AlreadyIncRefed<RegExp> re = RegExp::create(cx, str, flags);
if (!re)
return NULL;
JSObject *obj = NewBuiltinClassInstance(cx, &js_RegExpClass);
@ -435,7 +437,7 @@ RegExp::createObjectNoStatics(JSContext *cx, const jschar *chars, size_t length,
re->decref(cx);
return NULL;
}
obj->setPrivate(re);
obj->setPrivate(re.get());
obj->zeroRegExpLastIndex();
return obj;
}
@ -574,7 +576,7 @@ RegExp::extractFrom(JSObject *obj)
return re;
}
inline RegExp *
inline AlreadyIncRefed<RegExp>
RegExp::clone(JSContext *cx, const RegExp &other)
{
return create(cx, other.source, other.flags);

View File

@ -1576,32 +1576,30 @@ class FlatMatch
/* A regexp and optional associated object. */
class RegExpPair
{
AutoRefCount<RegExp> arc;
AutoRefCount<RegExp> re_;
JSObject *reobj_;
RegExp *re_;
explicit RegExpPair(RegExpPair &);
public:
explicit RegExpPair(JSContext *cx, RegExp *re) : arc(cx, re), re_(re) {}
explicit RegExpPair(JSContext *cx) : re_(cx) {}
void reset(JSObject &reobj) {
reobj_ = &reobj;
re_ = RegExp::extractFrom(&reobj);
JS_ASSERT(re_);
arc.reset(re_);
void reset(JSObject &obj) {
reobj_ = &obj;
RegExp *re = RegExp::extractFrom(reobj_);
JS_ASSERT(re);
re_.reset(NeedsIncRef<RegExp>(re));
}
void reset(RegExp &re) {
re_ = &re;
void reset(AlreadyIncRefed<RegExp> re) {
reobj_ = NULL;
arc.reset(re_);
re_.reset(re);
}
/* Note: May be null. */
JSObject *reobj() const { return reobj_; }
RegExp &re() const { JS_ASSERT(re_); return *re_; }
bool hasRegExp() const { return !!re_; }
bool hasRegExp() const { return !re_.null(); }
RegExp &re() const { JS_ASSERT(hasRegExp()); return *re_; }
};
/*
@ -1647,7 +1645,7 @@ class RegExpGuard
}
public:
explicit RegExpGuard(JSContext *cx) : cx(cx), rep(cx, NULL) {}
explicit RegExpGuard(JSContext *cx) : cx(cx), rep(cx) {}
~RegExpGuard() {}
/* init must succeed in order to call tryFlatMatch or normalizeRegExp. */
@ -1733,10 +1731,10 @@ class RegExpGuard
}
JS_ASSERT(patstr);
RegExp *re = RegExp::createFlagged(cx, patstr, opt);
AlreadyIncRefed<RegExp> re = RegExp::createFlagged(cx, patstr, opt);
if (!re)
return NULL;
rep.reset(*re);
rep.reset(re);
return &rep;
}