tree 1139baf2cfece78d665d86fa40cf64b4737b873d
parent 858e20a6334d4bb14b2d10bfa5cf8d4b2251b925
author Taylor Cramer <cramertj@pigweed.infra.roller.google.com> 1685039034 +0000
committer CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com> 1685039034 +0000

[roll third_party/pigweed] pw_rpc: Add destructor to stateful Call subclasses

b5f8cd9 moved waiting for callback completion from Call's destructors
into the destructors of some subclasses. This helped ensure that the
callbacks were completed prior to the destruction of fields referenced
by those callbacks.

However, that change missed a number of classes, and applied the
destructor to some classes where no extra state was carried (except
transitively through parent classes). This resulted in some bugs,
notably b/282138354.

This change moves the DestroyClientCall and DestroyServerCall
invocations into two places:
 - The destructors of any stateful Call subclasses (those subclasses
   whose fields may be referenced by the callbacks).
 - The destructors of ServerCall and ClientCall (to guard the stateful
   `Call` class appropriately in the case of classes whose heirarchy
   would not otherwise have invoked `DestroyXXXCall`).

This results in some duplication, notably between classes where multiple
layers in the inheritance tree are stateful: each stateful layer will
`DestroyXXXCall`. However, the alternative is leaving intermediate
classes unprotected, which makes it undefined behavior to create an
instance of such an intermediate class, and makes it hard to track
whether the final subclasses are properly protected: they must include a
destructor if and only if their parents had state and did *not* include
a destructor. This condition is much too complex to validate, especially
given that Call has 40+ subclasses, many with four-deep inheritance.

After this CL, it should be easy to check a class for correctness:
it must include a `DestroyXXXCall`-invoking destructor if it has a field
whose state may be referenced from a callback.

Original-Bug: b/282138354
Original-Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/148110

https://pigweed.googlesource.com/pigweed/pigweed
third_party/pigweed Rolled-Commits: 0d5b4d933a0c457..ea55bee9faa2b0f
Roller-URL: https://ci.chromium.org/b/8780104038322576993
GitWatcher: ignore
CQ-Do-Not-Cancel-Tryjobs: true
Change-Id: I195f2389285198b7477a353280ce400f2a93ded4
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/experimental/+/148934
Bot-Commit: Pigweed Integration Roller <pigweed-integration-roller@pigweed.google.com.iam.gserviceaccount.com>
Commit-Queue: Pigweed Integration Roller <pigweed-integration-roller@pigweed.google.com.iam.gserviceaccount.com>