On Jun 18, 2011, at 10:47 PM, Alexey Proskuryakov wrote:
>
> 18.06.2011, в 22:15, Maciej Stachowiak написал(а):
>
>> - I think having a rule for using PassRefPtr for arguments that depends on
>> how callers use the function is likely to be hard to use in practice, since
>> it requires global knowledge of all callers to pick the right function
>> signature. The rule I prefer is that if a function takes ownership of the
>> argument, whether or not we currently have a caller that gives away
>> ownership, the function should take PassRefPtr. If it does not take
>> ownership, it should take a raw pointer. That way you can decide the right
>> thing to do based solely on the definition of the function itself.
>
>
> Using PassRefPtr for arguments is a fairly common source of bugs. The way
> it's zeroed out after being used has caused crashes in code paths that
> weren't tested before commit for some reason (see e.g.
> <https://bugs.webkit.org/show_bug.cgi?id=52981>). Another example that we've
> just seen (<https://bugs.webkit.org/show_bug.cgi?id=62836>) was when a
> different compiler had a different order of evaluation, so building with it
> suddenly exposed a crash that we didn't need to have.
>
> Looking at either bug, using PassRefPtr is pointless. For trace(), we could
> theoretically pass ownership from jsConsolePrototypeFunctionTrace(), but we
> do not, and micro-optimizing Console functions for performance isn't worth
> the cost of having had this crasher bug in my opinion. For bug 62836, it's
> quite clearly impossible to pass ownership to HTMLTableRowsCollection.
Yes, the self-zeroing can cause bugs. But passing raw pointers can also cause
bugs, and possibly more serious ones because they'll be freed memory access
instead of null pointer derefs. I can see three options:
(1) Use PassRefPtr for every parameter that takes ownership.
Pro:
Bright-line rule; fairly clear how to apply it.
Distributed performance benefit.
Con:
Possible accidental self-zeroing bugs.
Maybe slight code bloat if it leads to reefing at call sites.
(2) Abandon PassRefPtr for incoming function arguments entirely.
Pro:
Extremely simple to apply the rule.
No accidental-self-zeroing bugs.
Con:
We probably lose some performance.
Possible accidental freed memory access bugs.
For cases where the object being passed really is freshly created, now you
have to make a local RefPtr variable to hold it.
(3) Use PassRefPtr based on whether callers ever provide a PassRefPtr as an
argument.
Pro:
If done right, get the performance benefit without the costs/hazards.
Con:
Applying this rule is hard; coding or reviewing a patch that changes
PassRefPtrness of an argument in principle requires checking every call site.
If people apply the rule wrong, we can get both null-deref and
freed-memory bugs.
(3) is my least favorite.
Maybe there are other options or other pros and cons. It might be useful to
make a wiki page if we want to collect more options and arguments.
Regards,
Maciej
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
