Gracefully handle child process death in out-of-process plugin loading.

Gracefully handle child process death in out-of-process plugin loading.

This also queues requests to load plugins, based on http://codereview.chromium.org/8243010/.

BUG=100053
TEST=Install Sonix webcam driver on OS X Lion and try to load a Flash video. It plays.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106738

[jam, 2011-10-17 23:08:38]

two comments:
-it makes things harder to review when you both move a file out, and also change
how it works. it would be much easier to review if you first moved it without
changing it, then that review would be a rubberstamp. then in another change you
actually change the behavior.
-you send the notification asynchronously and then load the next plugin. this is
inherently racy, since sending an IPC means that a task will be posted from the
main utility thread to its IO thread, and then that has to go through the pipe.
if during this time the next plugin loads and crashes, the browser will think
that the wrong plugin is at fault.

I'm hesitant to change these to sync IPCs, since that would  slow plugin loading
even further. maybe before we come up with any exotic solutions, you can time it
to see how long it takes with sync/async messages?

[Robert Sesek, 2011-10-18 00:01:30]

On 2011/10/17 23:08:38, John Abd-El-Malek wrote:
> two comments:
> -it makes things harder to review when you both move a file out, and also
change
> how it works. it would be much easier to review if you first moved it without
> changing it, then that review would be a rubberstamp. then in another change
you
> actually change the behavior.

Done. I had this as a separate git commit but forgot to pull it out as a
separate review. http://codereview.chromium.org/8334004/

> -you send the notification asynchronously and then load the next plugin. this
is
> inherently racy, since sending an IPC means that a task will be posted from
the
> main utility thread to its IO thread, and then that has to go through the
pipe.
> if during this time the next plugin loads and crashes, the browser will think
> that the wrong plugin is at fault.
> 
> I'm hesitant to change these to sync IPCs, since that would  slow plugin
loading
> even further. maybe before we come up with any exotic solutions, you can time
it
> to see how long it takes with sync/async messages?

Ah. I thought that because this code executed on the main thread it'd use the
sync channel that it has opened? Because UtilityThread::Send() operates on an
IPC::SyncChannel.

[Robert Sesek, 2011-10-18 17:00:26]

On 2011/10/18 00:01:30, rsesek wrote:
> On 2011/10/17 23:08:38, John Abd-El-Malek wrote:
> > -you send the notification asynchronously and then load the next plugin.
this
> is
> > inherently racy, since sending an IPC means that a task will be posted from
> the
> > main utility thread to its IO thread, and then that has to go through the
> pipe.
> > if during this time the next plugin loads and crashes, the browser will
think
> > that the wrong plugin is at fault.
> > 
> > I'm hesitant to change these to sync IPCs, since that would  slow plugin
> loading
> > even further. maybe before we come up with any exotic solutions, you can
time
> it
> > to see how long it takes with sync/async messages?
> 
> Ah. I thought that because this code executed on the main thread it'd use the
> sync channel that it has opened? Because UtilityThread::Send() operates on an
> IPC::SyncChannel.

Nevermind. I didn't realize that async messages were pushed to the IO thread
even if they were sent through the sync channel.

I also experimented with sync vs async messages and timing, and the results
fairly comparable. Using async messages, callbacks were ran after an avg. of
132,792µs (median=127,804µs). Using sync messages took an avg. of 137,134µs
(median=130,996µs). So the difference is roughly 3-4ms. I can share you the
spreadsheet if you're interested.

I think whichever we end up doing, we should use UMA histograms to see what the
real-world data is, because these tests were done on a beefy dev machine with a
warm disk cache.

This latest patch set switches to using sync messages.

[jam, 2011-10-18 20:49:46]

I haven't looked at the change in detail yet. But I had one question: would it
be simpler (and to avoid all the changes to PluginList) to tell the utility
process which plugins to not crawl instead of giving it the list of plugins to
crawl? the browser can know, as it does now, which plugins cause a crash. so the
second time it gets a list, it can given the utility process plugins to avoid.
that would be a simpler change to PluginList. wdyt?

[Robert Sesek, 2011-10-18 20:53:56]

On 2011/10/18 20:49:46, John Abd-El-Malek wrote:
> I haven't looked at the change in detail yet. But I had one question: would it
> be simpler (and to avoid all the changes to PluginList) to tell the utility
> process which plugins to not crawl instead of giving it the list of plugins to
> crawl? the browser can know, as it does now, which plugins cause a crash. so
the
> second time it gets a list, it can given the utility process plugins to avoid.
> that would be a simpler change to PluginList. wdyt?

How would the browser know which plugins cause a crash? Maybe I need more of an
explanation, but this does not sound simpler.

[jam, 2011-10-18 21:12:15]

On Tue, Oct 18, 2011 at 1:53 PM, <rsesek@chromium.org> wrote:

> On 2011/10/18 20:49:46, John Abd-El-Malek wrote:
>
>> I haven't looked at the change in detail yet. But I had one question:
>> would it
>> be simpler (and to avoid all the changes to PluginList) to tell the
>> utility
>> process which plugins to not crawl instead of giving it the list of
>> plugins to
>> crawl? the browser can know, as it does now, which plugins cause a crash.
>> so
>>
> the
>
>> second time it gets a list, it can given the utility process plugins to
>> avoid.
>> that would be a simpler change to PluginList. wdyt?
>>
>
> How would the browser know which plugins cause a crash? Maybe I need more
> of an
> explanation, but this does not sound simpler.
>

the utility process can send the browser each plugin it will crawl next. if
it crashes, then it knows which one caused it.

>
>
>
>
http://codereview.chromium.**org/8318028/<http://codereview.chromium.org/8318...
>

[Robert Sesek, 2011-10-18 21:46:09]

On 2011/10/18 21:12:15, John Abd-El-Malek wrote:
> On Tue, Oct 18, 2011 at 1:53 PM, <mailto:rsesek@chromium.org> wrote:
> 
> > On 2011/10/18 20:49:46, John Abd-El-Malek wrote:
> >
> >> I haven't looked at the change in detail yet. But I had one question:
> >> would it
> >> be simpler (and to avoid all the changes to PluginList) to tell the
> >> utility
> >> process which plugins to not crawl instead of giving it the list of
> >> plugins to
> >> crawl? the browser can know, as it does now, which plugins cause a crash.
> >> so
> >>
> > the
> >
> >> second time it gets a list, it can given the utility process plugins to
> >> avoid.
> >> that would be a simpler change to PluginList. wdyt?
> >>
> >
> > How would the browser know which plugins cause a crash? Maybe I need more
> > of an
> > explanation, but this does not sound simpler.
> >
> 
> the utility process can send the browser each plugin it will crawl next. if
> it crashes, then it knows which one caused it.

I understand now, but I think that this CL, while more complex in terms of CL
size/scope, is actually less complex in terms of being able to comprehend the
end result. (I also prefer the changes to PluginList in this CL compared to what
my previous CL added, GetPluginPathListsToLoad). Currently, the data being sent
across the process boundary is essentially the internal state of the PluginList,
which is then injected into another instance. I think it makes more conceptual
sense to send over a list of plugins to attempt to load and have the two
processes walk the list together.

What you're proposing also seems like more bookkeeping on the browser-side,
because if the process crashes, it would need to then send over a list of things
already loaded, a list of things to not load, and the utility would still end up
re-crawling directories it had already finished. It could probably be optimized
to be more efficient, but that's more bookkeeping.

[jam, 2011-10-18 22:03:21]

On Tue, Oct 18, 2011 at 2:46 PM, <rsesek@chromium.org> wrote:

> On 2011/10/18 21:12:15, John Abd-El-Malek wrote:
>
>  On Tue, Oct 18, 2011 at 1:53 PM, <mailto:rsesek@chromium.org> wrote:
>>
>
>  > On 2011/10/18 20:49:46, John Abd-El-Malek wrote:
>> >
>> >> I haven't looked at the change in detail yet. But I had one question:
>> >> would it
>> >> be simpler (and to avoid all the changes to PluginList) to tell the
>> >> utility
>> >> process which plugins to not crawl instead of giving it the list of
>> >> plugins to
>> >> crawl? the browser can know, as it does now, which plugins cause a
>> crash.
>> >> so
>> >>
>> > the
>> >
>> >> second time it gets a list, it can given the utility process plugins to
>> >> avoid.
>> >> that would be a simpler change to PluginList. wdyt?
>> >>
>> >
>> > How would the browser know which plugins cause a crash? Maybe I need
>> more
>> > of an
>> > explanation, but this does not sound simpler.
>> >
>>
>
>  the utility process can send the browser each plugin it will crawl next.
>> if
>> it crashes, then it knows which one caused it.
>>
>
> I understand now, but I think that this CL, while more complex in terms of
> CL
> size/scope, is actually less complex in terms of being able to comprehend
> the
> end result. (I also prefer the changes to PluginList in this CL compared to
> what
> my previous CL added, GetPluginPathListsToLoad). Currently, the data being
> sent
> across the process boundary is essentially the internal state of the
> PluginList,
> which is then injected into another instance. I think it makes more
> conceptual
> sense to send over a list of plugins to attempt to load and have the two
> processes walk the list together.
>
> What you're proposing also seems like more bookkeeping on the browser-side,
> because if the process crashes, it would need to then send over a list of
> things
> already loaded


this isn't necessary, they can be recrawled. this will be cheap since they
should already be in the cache, and it would be a rare event either way.


> , a list of things to not load, and the utility would still end up
> re-crawling directories it had already finished. It could probably be
> optimized
> to be more efficient, but that's more bookkeeping.
>

I was thinking of this to decrease bookkeeping :)


>
>
http://codereview.chromium.**org/8318028/<http://codereview.chromium.org/8318...
>

[Robert Sesek, 2011-10-18 22:32:13]

On 2011/10/18 22:03:21, John Abd-El-Malek wrote:
> this isn't necessary, they can be recrawled. this will be cheap since they
> should already be in the cache, and it would be a rare event either way.

This runs counter-intuitive to what we were discussing above regarding sync
messages since there's no guarantee that these items will still be in the disk
cache.

As I said, I prefer this approach; the amount of bookkeeping is reasonable and I
think it's easier to follow what the code is doing. I was never really pleased
with GetPluginPathsToLoad and SetPluginPathsToLoad.

[jam, 2011-10-20 00:16:16]

lgtm

sorry for the delay, I didn't see the update

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:52: DCHECK(process_host_);
nit: avoid this dcheck before variable use. in release build it does nothing. in
debug, you'll get a crash in the next line which will be just as useful

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:142: if (next_load_index_ >=
canonical_list_.size()) {
nit: better to early return instead to reduce tabbing

http://codereview.chromium.org/8318028/diff/4001/content/utility/utility_thre...
File content/utility/utility_thread_impl.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/utility/utility_thre...
content/utility/utility_thread_impl.cc:123: if (!plugin_groups.size()) {
nit: chrome style is if (plugin_groups.empty())

http://codereview.chromium.org/8318028/diff/4001/webkit/plugins/npapi/plugin_...
File webkit/plugins/npapi/plugin_list.cc (right):

http://codereview.chromium.org/8318028/diff/4001/webkit/plugins/npapi/plugin_...
webkit/plugins/npapi/plugin_list.cc:465: for (size_t i = 0; i <
extra_plugin_dirs.size(); ++i) {
nit: this file doesn't use brace brackets for one line statement

[Bernhard Bauer, 2011-10-20 08:20:04]

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:100: if (index >= canonical_list_.size())
When can that happen?

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:109: ++next_load_index_;
Maybe I'm just not understanding it, but why do we need the utility process to
send us the index with every message? The DCHECK seems to imply that we don't
expect any other index, so why don't we just remove the first element from
|canonical_list_| here?

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:129: internal_plugins_.begin();
Nit: one more space

[Robert Sesek, 2011-10-20 16:00:21]

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:52: DCHECK(process_host_);
On 2011/10/20 00:16:16, John Abd-El-Malek wrote:
> nit: avoid this dcheck before variable use. in release build it does nothing.
in
> debug, you'll get a crash in the next line which will be just as useful

Done.

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:100: if (index >= canonical_list_.size())
On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> When can that happen?

It's just being defensive against malformed/malicious messages.

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:109: ++next_load_index_;
On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> Maybe I'm just not understanding it, but why do we need the utility process to
> send us the index with every message? The DCHECK seems to imply that we don't
> expect any other index, so why don't we just remove the first element from
> |canonical_list_| here?

I thought about this but was hesitant because it would involve an array shift
each time (popping from the front). I measured this and it increases the
time-to-completion by 12.5ms. So, I'm keeping this as-is.

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:129: internal_plugins_.begin();
On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> Nit: one more space

Done.

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:142: if (next_load_index_ >=
canonical_list_.size()) {
On 2011/10/20 00:16:16, John Abd-El-Malek wrote:
> nit: better to early return instead to reduce tabbing

Done.

http://codereview.chromium.org/8318028/diff/4001/content/utility/utility_thre...
File content/utility/utility_thread_impl.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/utility/utility_thre...
content/utility/utility_thread_impl.cc:123: if (!plugin_groups.size()) {
On 2011/10/20 00:16:16, John Abd-El-Malek wrote:
> nit: chrome style is if (plugin_groups.empty())

Done.

http://codereview.chromium.org/8318028/diff/4001/webkit/plugins/npapi/plugin_...
File webkit/plugins/npapi/plugin_list.cc (right):

http://codereview.chromium.org/8318028/diff/4001/webkit/plugins/npapi/plugin_...
webkit/plugins/npapi/plugin_list.cc:465: for (size_t i = 0; i <
extra_plugin_dirs.size(); ++i) {
On 2011/10/20 00:16:16, John Abd-El-Malek wrote:
> nit: this file doesn't use brace brackets for one line statement

Done.

[Bernhard Bauer, 2011-10-20 16:15:07]

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:100: if (index >= canonical_list_.size())
On 2011/10/20 16:00:21, rsesek wrote:
> On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > When can that happen?
> 
> It's just being defensive against malformed/malicious messages.

Shouldn't we be more defensive in that case, i.e. killing the utility process?

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:109: ++next_load_index_;
On 2011/10/20 16:00:21, rsesek wrote:
> On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > Maybe I'm just not understanding it, but why do we need the utility process
to
> > send us the index with every message? The DCHECK seems to imply that we
don't
> > expect any other index, so why don't we just remove the first element from
> > |canonical_list_| here?
> 
> I thought about this but was hesitant because it would involve an array shift
> each time (popping from the front). I measured this and it increases the
> time-to-completion by 12.5ms. So, I'm keeping this as-is.

And if you use a linked list?

[Robert Sesek, 2011-10-20 16:22:36]

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:100: if (index >= canonical_list_.size())
On 2011/10/20 16:15:08, Bernhard Bauer wrote:
> On 2011/10/20 16:00:21, rsesek wrote:
> > On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > > When can that happen?
> > 
> > It's just being defensive against malformed/malicious messages.
> 
> Shouldn't we be more defensive in that case, i.e. killing the utility process?

Is there an API for that? I couldn't find one in ChildProcessHost.

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:109: ++next_load_index_;
On 2011/10/20 16:15:08, Bernhard Bauer wrote:
> On 2011/10/20 16:00:21, rsesek wrote:
> > On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > > Maybe I'm just not understanding it, but why do we need the utility
process
> to
> > > send us the index with every message? The DCHECK seems to imply that we
> don't
> > > expect any other index, so why don't we just remove the first element from
> > > |canonical_list_| here?
> > 
> > I thought about this but was hesitant because it would involve an array
shift
> > each time (popping from the front). I measured this and it increases the
> > time-to-completion by 12.5ms. So, I'm keeping this as-is.
> 
> And if you use a linked list?

I don't think we have param traits, and even if we did, it'd still be less
efficient because it'd need to make a bunch of small heap allocations for each
node (on the IO thread).

[Bernhard Bauer, 2011-10-20 16:27:56]

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:100: if (index >= canonical_list_.size())
On 2011/10/20 16:22:36, rsesek wrote:
> On 2011/10/20 16:15:08, Bernhard Bauer wrote:
> > On 2011/10/20 16:00:21, rsesek wrote:
> > > On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > > > When can that happen?
> > > 
> > > It's just being defensive against malformed/malicious messages.
> > 
> > Shouldn't we be more defensive in that case, i.e. killing the utility
process?
> 
> Is there an API for that? I couldn't find one in ChildProcessHost.

Not sure, but we do it for renderers and extension processes, right?

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:109: ++next_load_index_;
On 2011/10/20 16:22:36, rsesek wrote:
> On 2011/10/20 16:15:08, Bernhard Bauer wrote:
> > On 2011/10/20 16:00:21, rsesek wrote:
> > > On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > > > Maybe I'm just not understanding it, but why do we need the utility
> process
> > to
> > > > send us the index with every message? The DCHECK seems to imply that we
> > don't
> > > > expect any other index, so why don't we just remove the first element
from
> > > > |canonical_list_| here?
> > > 
> > > I thought about this but was hesitant because it would involve an array
> shift
> > > each time (popping from the front). I measured this and it increases the
> > > time-to-completion by 12.5ms. So, I'm keeping this as-is.
> > 
> > And if you use a linked list?
> 
> I don't think we have param traits, and even if we did, it'd still be less
> efficient because it'd need to make a bunch of small heap allocations for each
> node (on the IO thread).

Hm :-/

What if we just remove the index from the IPC message but keep counting
internally? If we're going to ignore out-of-order indices anyway, we might as
well not send them in the first place.

[Robert Sesek, 2011-10-20 16:30:28]

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:100: if (index >= canonical_list_.size())
On 2011/10/20 16:27:57, Bernhard Bauer wrote:
> On 2011/10/20 16:22:36, rsesek wrote:
> > On 2011/10/20 16:15:08, Bernhard Bauer wrote:
> > > On 2011/10/20 16:00:21, rsesek wrote:
> > > > On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > > > > When can that happen?
> > > > 
> > > > It's just being defensive against malformed/malicious messages.
> > > 
> > > Shouldn't we be more defensive in that case, i.e. killing the utility
> process?
> > 
> > Is there an API for that? I couldn't find one in ChildProcessHost.
> 
> Not sure, but we do it for renderers and extension processes, right?

Yes, but I think that may be a feature of RenderProcessHost.

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:109: ++next_load_index_;
On 2011/10/20 16:27:57, Bernhard Bauer wrote:
> On 2011/10/20 16:22:36, rsesek wrote:
> > On 2011/10/20 16:15:08, Bernhard Bauer wrote:
> > > On 2011/10/20 16:00:21, rsesek wrote:
> > > > On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > > > > Maybe I'm just not understanding it, but why do we need the utility
> > process
> > > to
> > > > > send us the index with every message? The DCHECK seems to imply that
we
> > > don't
> > > > > expect any other index, so why don't we just remove the first element
> from
> > > > > |canonical_list_| here?
> > > > 
> > > > I thought about this but was hesitant because it would involve an array
> > shift
> > > > each time (popping from the front). I measured this and it increases the
> > > > time-to-completion by 12.5ms. So, I'm keeping this as-is.
> > > 
> > > And if you use a linked list?
> > 
> > I don't think we have param traits, and even if we did, it'd still be less
> > efficient because it'd need to make a bunch of small heap allocations for
each
> > node (on the IO thread).
> 
> Hm :-/
> 
> What if we just remove the index from the IPC message but keep counting
> internally? If we're going to ignore out-of-order indices anyway, we might as
> well not send them in the first place.

What's wrong with sending the index?

[Robert Sesek, 2011-10-20 16:38:05]

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
File content/browser/plugin_loader_posix.cc (right):

http://codereview.chromium.org/8318028/diff/4001/content/browser/plugin_loade...
content/browser/plugin_loader_posix.cc:109: ++next_load_index_;
On 2011/10/20 16:30:28, rsesek wrote:
> On 2011/10/20 16:27:57, Bernhard Bauer wrote:
> > On 2011/10/20 16:22:36, rsesek wrote:
> > > On 2011/10/20 16:15:08, Bernhard Bauer wrote:
> > > > On 2011/10/20 16:00:21, rsesek wrote:
> > > > > On 2011/10/20 08:20:04, Bernhard Bauer wrote:
> > > > > > Maybe I'm just not understanding it, but why do we need the utility
> > > process
> > > > to
> > > > > > send us the index with every message? The DCHECK seems to imply that
> we
> > > > don't
> > > > > > expect any other index, so why don't we just remove the first
element
> > from
> > > > > > |canonical_list_| here?
> > > > > 
> > > > > I thought about this but was hesitant because it would involve an
array
> > > shift
> > > > > each time (popping from the front). I measured this and it increases
the
> > > > > time-to-completion by 12.5ms. So, I'm keeping this as-is.
> > > > 
> > > > And if you use a linked list?
> > > 
> > > I don't think we have param traits, and even if we did, it'd still be less
> > > efficient because it'd need to make a bunch of small heap allocations for
> each
> > > node (on the IO thread).
> > 
> > Hm :-/
> > 
> > What if we just remove the index from the IPC message but keep counting
> > internally? If we're going to ignore out-of-order indices anyway, we might
as
> > well not send them in the first place.
> 
> What's wrong with sending the index?

Actually, I now see that I don't need to do it because I can perform the sanity
check using the plugin path instead. Doing now.

[Robert Sesek, 2011-10-20 16:46:41]

OK. No longer sending the index.

[Bernhard Bauer, 2011-10-21 08:36:48]

On 2011/10/20 16:46:41, rsesek wrote:
> OK. No longer sending the index.

Woohoo! LGTM.