Skip to content

Commit

Permalink
Propagate user thrown exceptions.
Browse files Browse the repository at this point in the history
Updated `validate` to work with `toUserLand` to propagate exceptions
thrown bu the user, so that user thrown exceptions are not caught and
fed to the user callback as Strata errors.

Created a `toUserLand` function that will mark an exception thrown by
the user callback as such, so that it is not caught by `validate` and
fed back to the user callback. The user land exception is assigned to
the `thrownByUser` variable. As the stack unwinds, if a `validate`
function catch the exception, it will check to see if it is the same as
the `thrownByUser` function. If it is, it will rethrow the exception.

Note that this means we hold onto the last user exception thrown until
the next user exception is thrown. There's no way of knowing how deeply
nested we are in `validate` callbacks, so we can't clear it as part of
the unwind.

We clean it up periodically by clearing `thrownByUser` in calls to
`cursor`, which ought to get called often enough to ensure that the
memory is freed. Also, we clear in `close`, in case that's the last we
hear from the user for a while.

I can't imagine the trickery the user would employ to make a delayed
collection of a thrown error cause problems for his program. Be warned
that errors thrown through Strata are going to be held by Strata for
some time.

The alternative would be to set a flag on the error. No garbage
collection issues, but that's even more obtrusive. I'd prefer to be less
obtrusive, but holding onto an exception is not entirely unobtrusive.

It's not perfectly unobtrusive. I am not afraid that this is going to
cause problems for users, or that anyone is going to accuse me of
leaking memory. It's theoretically obtrusive, but practically
unobtrusive.

The only case that occurs to me is in ES6 where the user may be tracking
the exception using a `WeakMap` and counting on the disappearance of the
exception in the map to mean something. Except that I've had a look, if
the user does not hold a reference to the `Error`, he can't look in his
`WeakMap` for an error.

Thus, deterministic and unobtrusive. Thanks for listening.

Closes #174.
Closes #173.
  • Loading branch information
flatheadmill committed Aug 14, 2013
1 parent 87fccc1 commit 2912e87
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 17 deletions.
76 changes: 59 additions & 17 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ function Strata (options) {
return function (forward, type, report) { return validate(callback, forward, type, report) }
}

var thrownByUser

function validate (callback, forward, type, report) {
ok(typeof forward == "function", 'no forward function');
ok(typeof callback == "function",'no callback function');
Expand All @@ -331,12 +333,36 @@ function Strata (options) {
forward.apply(null, __slice.call(arguments, 1));
}
} catch (error) {
callback(error);
// Do not catch an exception thrown by the user.
if (thrownByUser === error) {
throw error
}
try {
callback(error);
} catch (e) {
// In this case, Strata has generated an error and the user wants to
// unwind the stack. It is a Strata internal error, but Strata
// should not catch it again.
thrownByUser = e
throw e
}
}
}
}
}

// When we a user's callback, we wrap our call. If the user has thrown an
// exception we make sure that `validate` will not catch it and feed it back
// to the user as a Strata generated Error.
function toUserLand (callback) {
try {
callback.apply(null, __slice.call(arguments, 1))
} catch (error) {
thrownByUser = error
throw error
}
}

function _size () { return size }

function _nextAddress () { return nextAddress }
Expand Down Expand Up @@ -1644,7 +1670,7 @@ function Strata (options) {
}

function replaced() {
if (--count == 0) callback();
if (--count == 0) toUserLand(callback);
}
}

Expand Down Expand Up @@ -1697,7 +1723,7 @@ function Strata (options) {
nextAddress = Math.max(+(file) + 1, nextAddress);
}
});
callback(null);
toUserLand(callback, null);
}
}

Expand All @@ -1706,7 +1732,14 @@ function Strata (options) {
// **TODO**: Need to actually purge cache and set sizes to zero.

// —
function close (callback) { callback(null) }
function close (callback) {
// In case this is the last we hear from the user.
thrownByUser = null;

// Nothing to do now, since we're always writing our pages. Eventually we're
// going to flush dirty pages or assert that there are no dirty pages.
toUserLand(callback, null);
}

// **TODO**: Close medic file.

Expand Down Expand Up @@ -2060,8 +2093,8 @@ function Strata (options) {

function unwind (callback) {
var vargs = [ null ].concat(__slice.call(arguments, 1));
// callback.apply(null, vargs);
process.nextTick(function () { callback.apply(null, vargs) });
callback.apply(null, vargs);
// process.nextTick(function () { callback.apply(null, vargs) });
}

// Binary search implemented, as always, by having a peek at [Algorithms in
Expand Down Expand Up @@ -2593,7 +2626,7 @@ function Strata (options) {
// Get a record at a given index from the current leaf page.
function get (index, callback) {
stash(page, index, validator(callback)(unstashed));
function unstashed (entry) { callback(null, entry.record) }
function unstashed (entry) { toUserLand(callback, null, entry.record) }
};

// Go to the next leaf page, the right sibling leaf page. Returns true if
Expand All @@ -2609,7 +2642,7 @@ function Strata (options) {
if (page.right) {
lock(page.right, exclusive, validate(callback, locked));
} else {
callback(null, false);
toUserLand(callback, null, false);
}

function locked (next) {
Expand All @@ -2624,7 +2657,7 @@ function Strata (options) {
length = page.positions.length;

// We have advanced.
callback(null, true);
toUserLand(callback, null, true);
}
}

Expand Down Expand Up @@ -2804,7 +2837,7 @@ function Strata (options) {
// are not the first leaf page, the record does not belong in this leaf
// page.
if (index == 0 && page.address != 1) {
callback(null, -1);
toUserLand(callback, null, -1);
return;
}

Expand Down Expand Up @@ -2868,7 +2901,7 @@ function Strata (options) {

function compare () {
if (comparator(key, rightLeafKey) < 0) insert();
else callback(null, +1);
else toUserLand(callback, null, +1);
}
}

Expand Down Expand Up @@ -2899,7 +2932,7 @@ function Strata (options) {
}

function close () {
callback(null, 0);
toUserLand(callback, null, 0);
}
}
}
Expand Down Expand Up @@ -2943,7 +2976,7 @@ function Strata (options) {
}

function closed () {
callback(null);
toUserLand(callback, null);
}
}

Expand Down Expand Up @@ -4995,6 +5028,9 @@ function Strata (options) {
function cursor (key, exclusive, callback) {
var sought, descent, check = validator(callback);

// As good a time as any to reset our user exception tracking.
thrownByUser = null;

// Descend to the penultimate branch page.
descent = new Descent();

Expand All @@ -5010,7 +5046,7 @@ function Strata (options) {
}

function leaf (page, index) {
callback(null, new Cursor(exclusive, key, page, index));
toUserLand(callback, null, new Cursor(exclusive, key, page, index));
}
}

Expand All @@ -5024,7 +5060,13 @@ function Strata (options) {
cursor(splat, true, splat.pop());
}

function balance (callback) { balancer.balance(callback) }
function balance (callback) {
balancer.balance(validate(callback, end));

function end () {
toUserLand(callback)
}
}

// Create an in memory mirror of a small b&#x2011tree for display. This is
// only intended for use against small trees for the sake of illustration.
Expand All @@ -5040,7 +5082,7 @@ function Strata (options) {
function begin (page) {
expand(page, root = page.addresses.map(record), 0, check(function () {
unlock(page);
callback(null, root);
toUserLand(callback, null, root);
}));
}

Expand All @@ -5049,7 +5091,7 @@ function Strata (options) {
var address = pages[index].address;
lock(address, false, check(address % 2 ? leaf : branch));
} else {
callback(null, pages);
toUserLand(callback, null, pages);
}

function branch (page) {
Expand Down
22 changes: 22 additions & 0 deletions t/coverage/propagation.t.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
require("./proof")(1, function (step, Strata, equal, tmp) {
var strata;
step(function () {

strata = new Strata({ directory: tmp, leafSize: 3, branchSize: 3 });
strata.create(step());

}, function () {

try {
strata.iterator('a', function (error, cursor) {
cursor.unlock()
throw new Error('propagated')
})
} catch (e) {
equal(e.message, 'propagated', 'propagated error')

strata.close(step())
}

});
});

0 comments on commit 2912e87

Please sign in to comment.