On Tue, Aug 1, 2017 at 12:31 PM, Alexis Beingessner
<a.beingess...@gmail.com> wrote:
> I was recently searching through our codebase to look at all the ways we
> use fallible allocations, and was startled when I came across several lines
> that looked like this:
>
> dom::SocketElement &mSocket = *sockets.AppendElement(fallible);
>
> For those who aren't familiar with how our allocating APIs work:
>
> * by default we hard abort on allocation failure
> * but if you add `fallible` (or use the Fallible template), you will get
> back nullptr on allocation failure
>
> So in isolation this code is saying "I want to handle allocation failure"
> and then immediately not doing that and just dereferencing the result. This
> turns allocation failure into Undefined Behaviour, rather than a process
> abort.
>
> Thankfully, all the cases where I found this were preceded by something
> like the following:
>
> uint32_t length = socketData->mData.Length();if
> (!sockets.SetCapacity(length, fallible)) { JS_ReportOutOfMemory(cx);
> return NS_ERROR_OUT_OF_MEMORY;}for (uint32_t i = 0; i <
> socketData->mData.Length(); i++) { dom::SocketElement &mSocket =
> *sockets.AppendElement(fallible);
>
> So really, the fallible AppendElement *is* infallible, but we're just
> saying "fallible" anyway.
Stating this upfront: I think we should use infallible AppendElement
here, or at the very least use a comment. But it's worth looking at
the larger context of one of these examples:
https://dxr.mozilla.org/mozilla-central/source/netwerk/base/Dashboard.cpp#426-435
(AFAICT, the only such examples of this pattern come from the above
file, and for the reasons outlined below.)
In particular, the API of Sequence<> is constrained because it
inherits from FallibleTArray, which *only* exposes fallible
operations. One can argue that FallibleTArray shouldn't do this, but
for Sequence, which is used for DOM bindings code, I believe the
intent is to nudge people writing DOM-exposed code to consider how to
recover from allocation failures, and to not blindly assume that
everything succeeds.
-Nathan
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
