Fix existing ref counting classes and add new ones.#13984
Fix existing ref counting classes and add new ones.#13984markdroth merged 5 commits intogrpc:masterfrom
Conversation
|
|
|
|
|
|
src/core/lib/support/orphanable.h
Outdated
|
|
||
| namespace grpc_core { | ||
|
|
||
| // A base class for orphanable objects. |
There was a problem hiding this comment.
Could you expand on the definition of "orphanable"?
src/core/lib/support/orphanable.h
Outdated
| class Orphanable { | ||
| public: | ||
| // Gives up ownership of the object. The implementation must arrange | ||
| // to destroy the object without further interaction from the caller. |
There was a problem hiding this comment.
I'd add "... must arrange to eventually destroy ..."
| }; | ||
|
|
||
| template <typename T, typename Deleter = OrphanableDelete<T>> | ||
| using OrphanablePtr = std::unique_ptr<T, Deleter>; |
There was a problem hiding this comment.
according to https://github.com/grpc/proposal/blob/master/L6-allow-c%2B%2B-in-grpc-core.md we shouldn't be using std::unique_ptr directly.
There was a problem hiding this comment.
The intent of that is that code outside of lib/support will use the abstractions provided by lib/support instead of directly using things provided in the standard library. But code in lib/support can use things provided by the standard library as long as we don't introduce a run-time dependency. In the case of std::unique_ptr<>, the only part that would impose a run-time dependency is the deleter, which we're overriding here.
Note that we do already define grpc_core::UniquePtr<>, which is basically defined as std::unique_ptr<> with a deleter that calls gpr_free():
| bool operator==(const RefCountedPtr& other) const { | ||
| return value_ == other.value_; | ||
| } | ||
| bool operator==(T* other) const { |
There was a problem hiding this comment.
you probably want to make the argument be const T* other. Same on line 88
|
|
|
|
Known issues: #13477 |
I am splitting this out of #13684 so that it can be used elsewhere.
Specific changes:
RefCountedandRefCountedWithTracingclasses.Orphanableclass and a subclass calledInternallyRefCountedWithTracing.