On Thu, Apr 21, 2016 at 8:41 PM, Martin Thomson <m...@mozilla.com> wrote:
>
>> - I suspect that in some heap-allocated objects it will be hard to do
>> the fallible parts without having an object of the relevant type. I
>> don't have specific examples, just a hunch.
>
> I'm not aware of any way that necessary state can't be fully
> established before constructing an object. If you find a way in which
> this is possible, please let me know.
I just tried this pattern in three classes I've been experimenting
with and hit problems in all three.
In bug 1265965 I wrote patches to convert nsThread+Init style to the
nsThread+outparam style, which worked well. So I tried the style
Nicolas and you are advocating, which I'll calls
"falllible-then-infallible". Here is the Init() function, from the
current trunk code:
nsresult
nsThread::Init()
{
// spawn thread and wait until it is fully setup
RefPtr<nsThreadStartupEvent> startup = new nsThreadStartupEvent();
NS_ADDREF_THIS();
mShutdownRequired = true;
// ThreadFunc is responsible for setting mThread
PRThread* thr = PR_CreateThread(PR_USER_THREAD, ThreadFunc, this,
PR_PRIORITY_NORMAL, PR_GLOBAL_THREAD,
PR_JOINABLE_THREAD, mStackSize);
if (!thr) {
NS_RELEASE_THIS();
return NS_ERROR_OUT_OF_MEMORY;
}
// ThreadFunc will wait for this event to be run before it tries to access
// mThread. By delaying insertion of this event into the queue, we ensure
// that mThread is set properly.
{
MutexAutoLock lock(mLock);
mEventsRoot.PutEvent(startup, lock); // retain a reference
}
// Wait for thread to call ThreadManager::SetupCurrentThread, which completes
// initialization of ThreadFunc.
startup->Wait();
return NS_OK;
}
So this is the code that must run before we construct an actual
nsThread object, right? The PR_CreateThread() call is the main
problem. It takes |this| as a parameter and proceeds to do all sorts
of stuff with it. It's probably not *impossible* to rewrite this as
fallible-then-infallible, but I'm struggling to see what it would look
like.
The second example I looked at was nsScriptSecurityManager. The
constructor+outparam style worked well there, but
fallible-then-infallible again hits a problem -- InitPrefs() is called
in the fallible section of initialization, and it calls
ScriptSecurityPrefChanged(), which touches lots of class members.
Maybe InitPrefs() could be moved out of the fallible section, but I
don't know this code well enough to say one way or the other.
The third example I looked at was CycleCollectedJSRuntime. Again
problems, specifically this line in Init():
mOwningThread->SetScriptObserver(this);
So, for all three examples I looked at fallible-then-infallible had
problems ranging from moderate to severe. My conclusion is that
partially initializing an object when the object doesn't exist is
difficult, and, as a result, fallible-then-infallible is not a
reliable general-purpose pattern.
Nick
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
