diff --git a/.github/mergeable.yml b/.github/mergeable.yml index f0180b99798..660d8cb4405 100644 --- a/.github/mergeable.yml +++ b/.github/mergeable.yml @@ -2,5 +2,5 @@ mergeable: pull_requests: label: must_include: - regex: "release notes:yes|release notes:no" + regex: "release notes: yes|release notes: no" message: "Add release notes yes/no label. For yes, add lang label" diff --git a/doc/server-reflection.md b/doc/server-reflection.md index cceee1647f6..c9b476599f1 100644 --- a/doc/server-reflection.md +++ b/doc/server-reflection.md @@ -181,3 +181,16 @@ will need to index those FileDescriptorProtos by file and symbol and imports. One issue is that some grpc implementations are very loosely coupled with protobufs; in such implementations it probably makes sense to split apart these reflection APIs so as not to take an additional proto dependency. + +## Known Implementations + +Enabling server reflection differs language-to-language. Here are links to docs relevant to +each language: + +- [Java](https://github.com/grpc/grpc-java/blob/master/documentation/server-reflection-tutorial.md#enable-server-reflection) +- [Go](https://github.com/grpc/grpc-go/blob/master/Documentation/server-reflection-tutorial.md#enable-server-reflection) +- [C++](https://grpc.io/grpc/cpp/md_doc_server_reflection_tutorial.html) +- [C#](https://github.com/grpc/grpc/blob/master/doc/csharp/server_reflection.md) +- Python: (tutorial not yet written) +- Ruby: not yet implemented [#2567](https://github.com/grpc/grpc/issues/2567) +- Node: not yet implemented [#2568](https://github.com/grpc/grpc/issues/2568) diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.cc b/src/core/ext/filters/client_channel/client_channel_channelz.cc index 4c9c9a6bd66..86c765df521 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.cc +++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc @@ -105,8 +105,8 @@ grpc_arg ClientChannelNode::CreateChannelArg() { RefCountedPtr ClientChannelNode::MakeClientChannelNode( grpc_channel* channel, size_t channel_tracer_max_nodes, bool is_top_level_channel) { - return MakePolymorphicRefCounted( - channel, channel_tracer_max_nodes, is_top_level_channel); + return MakeRefCounted(channel, channel_tracer_max_nodes, + is_top_level_channel); } } // namespace channelz diff --git a/src/core/lib/gprpp/orphanable.h b/src/core/lib/gprpp/orphanable.h index d0ec9b6461d..3123e3f5a39 100644 --- a/src/core/lib/gprpp/orphanable.h +++ b/src/core/lib/gprpp/orphanable.h @@ -86,7 +86,8 @@ class InternallyRefCounted : public Orphanable { GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE // Allow RefCountedPtr<> to access Unref() and IncrementRefCount(). - friend class RefCountedPtr; + template + friend class RefCountedPtr; InternallyRefCounted() { gpr_ref_init(&refs_, 1); } virtual ~InternallyRefCounted() {} @@ -129,7 +130,8 @@ class InternallyRefCountedWithTracing : public Orphanable { GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE // Allow RefCountedPtr<> to access Unref() and IncrementRefCount(). - friend class RefCountedPtr; + template + friend class RefCountedPtr; InternallyRefCountedWithTracing() : InternallyRefCountedWithTracing(static_cast(nullptr)) {} diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h index ddac5bd4755..03c293f6ed9 100644 --- a/src/core/lib/gprpp/ref_counted.h +++ b/src/core/lib/gprpp/ref_counted.h @@ -73,7 +73,8 @@ class RefCounted { private: // Allow RefCountedPtr<> to access IncrementRefCount(). - friend class RefCountedPtr; + template + friend class RefCountedPtr; void IncrementRefCount() { gpr_ref(&refs_); } @@ -152,7 +153,8 @@ class RefCountedWithTracing { private: // Allow RefCountedPtr<> to access IncrementRefCount(). - friend class RefCountedPtr; + template + friend class RefCountedPtr; void IncrementRefCount() { gpr_ref(&refs_); } diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h index 534d3d03cb4..c2dfbdd90f0 100644 --- a/src/core/lib/gprpp/ref_counted_ptr.h +++ b/src/core/lib/gprpp/ref_counted_ptr.h @@ -36,25 +36,49 @@ class RefCountedPtr { RefCountedPtr(std::nullptr_t) {} // If value is non-null, we take ownership of a ref to it. - explicit RefCountedPtr(T* value) { value_ = value; } + template + explicit RefCountedPtr(Y* value) { + value_ = value; + } - // Move support. + // Move ctors. RefCountedPtr(RefCountedPtr&& other) { value_ = other.value_; other.value_ = nullptr; } + template + RefCountedPtr(RefCountedPtr&& other) { + value_ = other.value_; + other.value_ = nullptr; + } + + // Move assignment. RefCountedPtr& operator=(RefCountedPtr&& other) { if (value_ != nullptr) value_->Unref(); value_ = other.value_; other.value_ = nullptr; return *this; } + template + RefCountedPtr& operator=(RefCountedPtr&& other) { + if (value_ != nullptr) value_->Unref(); + value_ = other.value_; + other.value_ = nullptr; + return *this; + } - // Copy support. + // Copy ctors. RefCountedPtr(const RefCountedPtr& other) { if (other.value_ != nullptr) other.value_->IncrementRefCount(); value_ = other.value_; } + template + RefCountedPtr(const RefCountedPtr& other) { + if (other.value_ != nullptr) other.value_->IncrementRefCount(); + value_ = other.value_; + } + + // Copy assignment. RefCountedPtr& operator=(const RefCountedPtr& other) { // Note: Order of reffing and unreffing is important here in case value_ // and other.value_ are the same object. @@ -63,17 +87,32 @@ class RefCountedPtr { value_ = other.value_; return *this; } + template + RefCountedPtr& operator=(const RefCountedPtr& other) { + // Note: Order of reffing and unreffing is important here in case value_ + // and other.value_ are the same object. + if (other.value_ != nullptr) other.value_->IncrementRefCount(); + if (value_ != nullptr) value_->Unref(); + value_ = other.value_; + return *this; + } ~RefCountedPtr() { if (value_ != nullptr) value_->Unref(); } // If value is non-null, we take ownership of a ref to it. - void reset(T* value = nullptr) { + template + void reset(Y* value) { if (value_ != nullptr) value_->Unref(); value_ = value; } + void reset() { + if (value_ != nullptr) value_->Unref(); + value_ = nullptr; + } + // TODO(roth): This method exists solely as a transition mechanism to allow // us to pass a ref to idiomatic C code that does not use RefCountedPtr<>. // Once all of our code has been converted to idiomatic C++, this @@ -89,16 +128,34 @@ class RefCountedPtr { T& operator*() const { return *value_; } T* operator->() const { return value_; } - bool operator==(const RefCountedPtr& other) const { + template + bool operator==(const RefCountedPtr& other) const { return value_ == other.value_; } - bool operator==(const T* other) const { return value_ == other; } - bool operator!=(const RefCountedPtr& other) const { + + template + bool operator==(const Y* other) const { + return value_ == other; + } + + bool operator==(std::nullptr_t) const { return value_ == nullptr; } + + template + bool operator!=(const RefCountedPtr& other) const { return value_ != other.value_; } - bool operator!=(const T* other) const { return value_ != other; } + + template + bool operator!=(const Y* other) const { + return value_ != other; + } + + bool operator!=(std::nullptr_t) const { return value_ != nullptr; } private: + template + friend class RefCountedPtr; + T* value_ = nullptr; }; @@ -107,11 +164,6 @@ inline RefCountedPtr MakeRefCounted(Args&&... args) { return RefCountedPtr(New(std::forward(args)...)); } -template -inline RefCountedPtr MakePolymorphicRefCounted(Args&&... args) { - return RefCountedPtr(New(std::forward(args)...)); -} - } // namespace grpc_core #endif /* GRPC_CORE_LIB_GPRPP_REF_COUNTED_PTR_H */ diff --git a/src/core/lib/iomgr/ev_poll_posix.cc b/src/core/lib/iomgr/ev_poll_posix.cc index 900e048d516..fb4c71ef71b 100644 --- a/src/core/lib/iomgr/ev_poll_posix.cc +++ b/src/core/lib/iomgr/ev_poll_posix.cc @@ -532,8 +532,10 @@ static void fd_notify_on_write(grpc_fd* fd, grpc_closure* closure) { } static void fd_notify_on_error(grpc_fd* fd, grpc_closure* closure) { - gpr_log(GPR_ERROR, "Polling engine does not support tracking errors."); - abort(); + if (grpc_polling_trace.enabled()) { + gpr_log(GPR_ERROR, "Polling engine does not support tracking errors."); + } + GRPC_CLOSURE_SCHED(closure, GRPC_ERROR_CANCELLED); } static void fd_set_readable(grpc_fd* fd) { diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 847bb87f7e5..dbad5ded4d4 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -529,6 +529,7 @@ void grpc_call_internal_unref(grpc_call* c REF_ARG) { static void release_call(void* call, grpc_error* error) { grpc_call* c = static_cast(call); grpc_channel* channel = c->channel; + gpr_free(static_cast(const_cast(c->final_info.error_string))); grpc_call_combiner_destroy(&c->call_combiner); grpc_channel_update_call_size_estimate(channel, gpr_arena_destroy(c->arena)); GRPC_CHANNEL_INTERNAL_UNREF(channel, "call"); @@ -573,7 +574,6 @@ static void destroy_call(void* call, grpc_error* error) { grpc_call_stack_destroy(CALL_STACK_FROM_CALL(c), &c->final_info, GRPC_CLOSURE_INIT(&c->release_call, release_call, c, grpc_schedule_on_exec_ctx)); - gpr_free(static_cast(const_cast(c->final_info.error_string))); } void grpc_call_ref(grpc_call* c) { gpr_ref(&c->ext_ref); } diff --git a/src/objective-c/tests/build_one_example.sh b/src/objective-c/tests/build_one_example.sh index 1eace541e6c..084147f1d46 100755 --- a/src/objective-c/tests/build_one_example.sh +++ b/src/objective-c/tests/build_one_example.sh @@ -43,7 +43,7 @@ xcodebuild \ -workspace *.xcworkspace \ -scheme $SCHEME \ -destination generic/platform=iOS \ - -derivedDataPath Build \ + -derivedDataPath Build/Build \ CODE_SIGN_IDENTITY="" \ CODE_SIGNING_REQUIRED=NO \ | egrep -v "$XCODEBUILD_FILTER" \ diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index 99d9a4847fd..f224457a557 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -187,8 +187,8 @@ TEST_P(ChannelTracerTest, ComplexTest) { AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); - sc1.reset(nullptr); - sc2.reset(nullptr); + sc1.reset(); + sc2.reset(); } // Test a case in which the parent channel has subchannels and the subchannels @@ -234,9 +234,9 @@ TEST_P(ChannelTracerTest, TestNesting) { grpc_slice_from_static_string("subchannel one inactive"), sc1); AddSimpleTrace(&tracer); ValidateChannelTrace(&tracer, 8, GetParam()); - sc1.reset(nullptr); - sc2.reset(nullptr); - conn1.reset(nullptr); + sc1.reset(); + sc2.reset(); + conn1.reset(); } INSTANTIATE_TEST_CASE_P(ChannelTracerTestSweep, ChannelTracerTest, diff --git a/test/core/gprpp/ref_counted_ptr_test.cc b/test/core/gprpp/ref_counted_ptr_test.cc index aa30b72282f..463b5e89667 100644 --- a/test/core/gprpp/ref_counted_ptr_test.cc +++ b/test/core/gprpp/ref_counted_ptr_test.cc @@ -127,7 +127,7 @@ TEST(RefCountedPtr, ResetFromNonNullToNull) { TEST(RefCountedPtr, ResetFromNullToNull) { RefCountedPtr foo; EXPECT_EQ(nullptr, foo.get()); - foo.reset(nullptr); + foo.reset(); EXPECT_EQ(nullptr, foo.get()); } @@ -175,6 +175,67 @@ TEST(RefCountedPtr, RefCountedWithTracing) { foo->Unref(DEBUG_LOCATION, "foo"); } +class BaseClass : public RefCounted { + public: + BaseClass() {} +}; + +class Subclass : public BaseClass { + public: + Subclass() {} +}; + +TEST(RefCountedPtr, ConstructFromSubclass) { + RefCountedPtr p(New()); +} + +TEST(RefCountedPtr, CopyAssignFromSubclass) { + RefCountedPtr b; + EXPECT_EQ(nullptr, b.get()); + RefCountedPtr s = MakeRefCounted(); + b = s; + EXPECT_NE(nullptr, b.get()); +} + +TEST(RefCountedPtr, MoveAssignFromSubclass) { + RefCountedPtr b; + EXPECT_EQ(nullptr, b.get()); + RefCountedPtr s = MakeRefCounted(); + b = std::move(s); + EXPECT_NE(nullptr, b.get()); +} + +TEST(RefCountedPtr, ResetFromSubclass) { + RefCountedPtr b; + EXPECT_EQ(nullptr, b.get()); + b.reset(New()); + EXPECT_NE(nullptr, b.get()); +} + +TEST(RefCountedPtr, EqualityWithSubclass) { + Subclass* s = New(); + RefCountedPtr b(s); + EXPECT_EQ(b, s); +} + +void FunctionTakingBaseClass(RefCountedPtr p) { + p.reset(); // To appease clang-tidy. +} + +TEST(RefCountedPtr, CanPassSubclassToFunctionExpectingBaseClass) { + RefCountedPtr p = MakeRefCounted(); + FunctionTakingBaseClass(p); +} + +void FunctionTakingSubclass(RefCountedPtr p) { + p.reset(); // To appease clang-tidy. +} + +TEST(RefCountedPtr, CanPassSubclassToFunctionExpectingSubclass) { + RefCountedPtr p = MakeRefCounted(); + FunctionTakingSubclass(p); +} + } // namespace } // namespace testing } // namespace grpc_core diff --git a/tools/internal_ci/helper_scripts/prepare_build_macos_rc b/tools/internal_ci/helper_scripts/prepare_build_macos_rc index bc7fff1b144..cbc42f52956 100644 --- a/tools/internal_ci/helper_scripts/prepare_build_macos_rc +++ b/tools/internal_ci/helper_scripts/prepare_build_macos_rc @@ -89,3 +89,7 @@ export DOTNET_CLI_TELEMETRY_OPTOUT=true date git submodule update --init + +# Store intermediate build files of ios binary size test into /tmpfs +mkdir /tmpfs/Build-ios-binary-size +ln -s /tmpfs/Build-ios-binary-size src/objective-c/examples/Sample/Build diff --git a/tools/profiling/ios_bin/binary_size.py b/tools/profiling/ios_bin/binary_size.py index b07adb57345..91c43f44243 100755 --- a/tools/profiling/ios_bin/binary_size.py +++ b/tools/profiling/ios_bin/binary_size.py @@ -55,7 +55,7 @@ def dir_size(dir): def get_size(where, frameworks): - build_dir = 'src/objective-c/examples/Sample/Build-%s/' % where + build_dir = 'src/objective-c/examples/Sample/Build/Build-%s/' % where if not frameworks: link_map_filename = 'Build/Intermediates.noindex/Sample.build/Release-iphoneos/Sample.build/Sample-LinkMap-normal-arm64.txt' return parse_link_map(build_dir + link_map_filename) @@ -76,14 +76,15 @@ def get_size(where, frameworks): def build(where, frameworks): shutil.rmtree( - 'src/objective-c/examples/Sample/Build-%s' % where, ignore_errors=True) + 'src/objective-c/examples/Sample/Build/Build-%s' % where, + ignore_errors=True) subprocess.check_call( 'CONFIG=opt EXAMPLE_PATH=src/objective-c/examples/Sample SCHEME=Sample FRAMEWORKS=%s ./build_one_example.sh' % ('YES' if frameworks else 'NO'), shell=True, cwd='src/objective-c/tests') - os.rename('src/objective-c/examples/Sample/Build', - 'src/objective-c/examples/Sample/Build-%s' % where) + os.rename('src/objective-c/examples/Sample/Build/Build', + 'src/objective-c/examples/Sample/Build/Build-%s' % where) text = 'Objective-C binary sizes\n'