Bug 1243242 - Don't make structured cloning O(n**2) in the size of the transferables array. r=sfink

This commit is contained in:
Jeff Walden 2016-01-27 12:29:17 -08:00
parent 83cfb1f226
commit 21a6a80d13
2 changed files with 59 additions and 17 deletions

View File

@ -0,0 +1,25 @@
// |reftest| skip-if(!xulRuntime.shell) -- requires serialize()
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
function test()
{
// On my system, with an unfixed build where transfer-list processing is
// quadratic, 5e5 elements makes this test take ~70s in a shell opt build.
// Debug build is well into timeout-land at 300+s. As long as at least *one*
// platform times out for a quadratic algorithm, a regression should be
// obvious. (Time to run the test in even a debug shell is ~17s, well short
// of timing out.)
var transfers = [];
for (var i = 0; i < 5e5; i++)
transfers.push(new ArrayBuffer());
// If serialization is quadratic in the length of |transfers|, the test will
// time out. If the test doesn't time out, it passed.
serialize({}, transfers);
}
test();
if (typeof reportCompare === "function")
reportCompare(0, 0, 'ok');

View File

@ -278,7 +278,8 @@ struct JSStructuredCloneWriter {
: out(cx), objs(out.context()),
counts(out.context()), entries(out.context()),
memory(out.context(), CloneMemory(out.context())), callbacks(cb),
closure(cbClosure), transferable(out.context(), tVal), transferableObjects(out.context())
closure(cbClosure), transferable(out.context(), tVal),
transferableObjects(out.context(), GCHashSet<JSObject*>(cx))
{}
~JSStructuredCloneWriter();
@ -349,9 +350,9 @@ struct JSStructuredCloneWriter {
// Any value passed to JS_WriteStructuredClone.
void* closure;
// List of transferable objects
// Set of transferable objects
RootedValue transferable;
AutoObjectVector transferableObjects;
Rooted<GCHashSet<JSObject*>> transferableObjects;
friend bool JS_WriteString(JSStructuredCloneWriter* w, HandleString str);
friend bool JS_WriteTypedArray(JSStructuredCloneWriter* w, HandleValue v);
@ -765,10 +766,14 @@ JSStructuredCloneWriter::~JSStructuredCloneWriter()
bool
JSStructuredCloneWriter::parseTransferable()
{
MOZ_ASSERT(transferableObjects.empty(), "parseTransferable called with stale data");
// NOTE: The transferables set is tested for non-emptiness at various
// junctures in structured cloning, so this set must be initialized
// by this method in all non-error cases.
MOZ_ASSERT(!transferableObjects.initialized(),
"parseTransferable called with stale data");
if (transferable.isNull() || transferable.isUndefined())
return true;
return transferableObjects.init(0);
if (!transferable.isObject())
return reportErrorTransferable(JS_SCERR_TRANSFERABLE);
@ -782,26 +787,36 @@ JSStructuredCloneWriter::parseTransferable()
return reportErrorTransferable(JS_SCERR_TRANSFERABLE);
uint32_t length;
if (!JS_GetArrayLength(cx, array, &length)) {
if (!JS_GetArrayLength(cx, array, &length))
return false;
}
// Initialize the set for the provided array's length.
if (!transferableObjects.init(length))
return false;
if (length == 0)
return true;
RootedValue v(context());
RootedObject tObj(context());
for (uint32_t i = 0; i < length; ++i) {
if (!CheckForInterrupt(cx))
return false;
if (!JS_GetElement(cx, array, i, &v))
return false;
if (!v.isObject())
return reportErrorTransferable(JS_SCERR_TRANSFERABLE);
RootedObject tObj(context(), &v.toObject());
tObj = &v.toObject();
// No duplicates allowed
if (std::find(transferableObjects.begin(), transferableObjects.end(), tObj) != transferableObjects.end()) {
auto p = transferableObjects.lookupForAdd(tObj);
if (p)
return reportErrorTransferable(JS_SCERR_DUP_TRANSFERABLE);
}
if (!transferableObjects.append(tObj))
if (!transferableObjects.add(p, tObj))
return false;
}
@ -1227,11 +1242,12 @@ JSStructuredCloneWriter::writeTransferMap()
if (!out.writePair(SCTAG_TRANSFER_MAP_HEADER, (uint32_t)SCTAG_TM_UNREAD))
return false;
if (!out.write(transferableObjects.length()))
if (!out.write(transferableObjects.count()))
return false;
for (JS::AutoObjectVector::Range tr = transferableObjects.all(); !tr.empty(); tr.popFront()) {
JSObject* obj = tr.front();
RootedObject obj(context());
for (auto tr = transferableObjects.all(); !tr.empty(); tr.popFront()) {
obj = tr.front();
if (!memory.put(obj, memory.count()))
return false;
@ -1261,11 +1277,12 @@ JSStructuredCloneWriter::transferOwnership()
uint64_t* point = out.rawBuffer();
MOZ_ASSERT(uint32_t(LittleEndian::readUint64(point) >> 32) == SCTAG_TRANSFER_MAP_HEADER);
point++;
MOZ_ASSERT(LittleEndian::readUint64(point) == transferableObjects.length());
MOZ_ASSERT(LittleEndian::readUint64(point) == transferableObjects.count());
point++;
for (JS::AutoObjectVector::Range tr = transferableObjects.all(); !tr.empty(); tr.popFront()) {
RootedObject obj(context(), tr.front());
RootedObject obj(context());
for (auto tr = transferableObjects.all(); !tr.empty(); tr.popFront()) {
obj = tr.front();
uint32_t tag;
JS::TransferableOwnership ownership;