Merge branch 'master' into pollforceset

.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"

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)

src/core/ext/filters/client_channel/client_channel_channelz.cc

@@ -105,8 +105,8 @@ grpc_arg ClientChannelNode::CreateChannelArg() {
 RefCountedPtr<ChannelNode> ClientChannelNode::MakeClientChannelNode(
     grpc_channel* channel, size_t channel_tracer_max_nodes,
     bool is_top_level_channel) {
-  return MakePolymorphicRefCounted<ChannelNode, ClientChannelNode>(
-      channel, channel_tracer_max_nodes, is_top_level_channel);
+  return MakeRefCounted<ClientChannelNode>(channel, channel_tracer_max_nodes,
+                                           is_top_level_channel);
 }
 
 }  // namespace channelz

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<Child>;
+  template <typename T>
+  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<Child>;
+  template <typename T>
+  friend class RefCountedPtr;
 
   InternallyRefCountedWithTracing()
       : InternallyRefCountedWithTracing(static_cast<TraceFlag*>(nullptr)) {}

src/core/lib/gprpp/ref_counted.h

@@ -73,7 +73,8 @@ class RefCounted {
 
  private:
   // Allow RefCountedPtr<> to access IncrementRefCount().
-  friend class RefCountedPtr<Child>;
+  template <typename T>
+  friend class RefCountedPtr;
 
   void IncrementRefCount() { gpr_ref(&refs_); }
 
@@ -152,7 +153,8 @@ class RefCountedWithTracing {
 
  private:
   // Allow RefCountedPtr<> to access IncrementRefCount().
-  friend class RefCountedPtr<Child>;
+  template <typename T>
+  friend class RefCountedPtr;
 
   void IncrementRefCount() { gpr_ref(&refs_); }
 

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 <typename Y>
+  explicit RefCountedPtr(Y* value) {
+    value_ = value;
+  }
 
-  // Move support.
+  // Move ctors.
   RefCountedPtr(RefCountedPtr&& other) {
     value_ = other.value_;
     other.value_ = nullptr;
   }
+  template <typename Y>
+  RefCountedPtr(RefCountedPtr<Y>&& 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 <typename Y>
+  RefCountedPtr& operator=(RefCountedPtr<Y>&& 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 <typename Y>
+  RefCountedPtr(const RefCountedPtr<Y>& 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 <typename Y>
+  RefCountedPtr& operator=(const RefCountedPtr<Y>& 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 <typename Y>
+  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 <typename Y>
+  bool operator==(const RefCountedPtr<Y>& other) const {
     return value_ == other.value_;
   }
-  bool operator==(const T* other) const { return value_ == other; }
-  bool operator!=(const RefCountedPtr& other) const {
+
+  template <typename Y>
+  bool operator==(const Y* other) const {
+    return value_ == other;
+  }
+
+  bool operator==(std::nullptr_t) const { return value_ == nullptr; }
+
+  template <typename Y>
+  bool operator!=(const RefCountedPtr<Y>& other) const {
     return value_ != other.value_;
   }
-  bool operator!=(const T* other) const { return value_ != other; }
+
+  template <typename Y>
+  bool operator!=(const Y* other) const {
+    return value_ != other;
+  }
+
+  bool operator!=(std::nullptr_t) const { return value_ != nullptr; }
 
  private:
+  template <typename Y>
+  friend class RefCountedPtr;
+
   T* value_ = nullptr;
 };
 
@@ -107,11 +164,6 @@ inline RefCountedPtr<T> MakeRefCounted(Args&&... args) {
   return RefCountedPtr<T>(New<T>(std::forward<Args>(args)...));
 }
 
-template <typename Parent, typename Child, typename... Args>
-inline RefCountedPtr<Parent> MakePolymorphicRefCounted(Args&&... args) {
-  return RefCountedPtr<Parent>(New<Child>(std::forward<Args>(args)...));
-}
-
 }  // namespace grpc_core
 
 #endif /* GRPC_CORE_LIB_GPRPP_REF_COUNTED_PTR_H */

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) {
+  if (grpc_polling_trace.enabled()) {
     gpr_log(GPR_ERROR, "Polling engine does not support tracking errors.");
-  abort();
+  }
+  GRPC_CLOSURE_SCHED(closure, GRPC_ERROR_CANCELLED);
 }
 
 static void fd_set_readable(grpc_fd* fd) {

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<grpc_call*>(call);
   grpc_channel* channel = c->channel;
+  gpr_free(static_cast<void*>(const_cast<char*>(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<void*>(const_cast<char*>(c->final_info.error_string)));
 }
 
 void grpc_call_ref(grpc_call* c) { gpr_ref(&c->ext_ref); }

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" \

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,

test/core/gprpp/ref_counted_ptr_test.cc

@@ -127,7 +127,7 @@ TEST(RefCountedPtr, ResetFromNonNullToNull) {
 TEST(RefCountedPtr, ResetFromNullToNull) {
   RefCountedPtr<Foo> 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<BaseClass> {
+ public:
+  BaseClass() {}
+};
+
+class Subclass : public BaseClass {
+ public:
+  Subclass() {}
+};
+
+TEST(RefCountedPtr, ConstructFromSubclass) {
+  RefCountedPtr<BaseClass> p(New<Subclass>());
+}
+
+TEST(RefCountedPtr, CopyAssignFromSubclass) {
+  RefCountedPtr<BaseClass> b;
+  EXPECT_EQ(nullptr, b.get());
+  RefCountedPtr<Subclass> s = MakeRefCounted<Subclass>();
+  b = s;
+  EXPECT_NE(nullptr, b.get());
+}
+
+TEST(RefCountedPtr, MoveAssignFromSubclass) {
+  RefCountedPtr<BaseClass> b;
+  EXPECT_EQ(nullptr, b.get());
+  RefCountedPtr<Subclass> s = MakeRefCounted<Subclass>();
+  b = std::move(s);
+  EXPECT_NE(nullptr, b.get());
+}
+
+TEST(RefCountedPtr, ResetFromSubclass) {
+  RefCountedPtr<BaseClass> b;
+  EXPECT_EQ(nullptr, b.get());
+  b.reset(New<Subclass>());
+  EXPECT_NE(nullptr, b.get());
+}
+
+TEST(RefCountedPtr, EqualityWithSubclass) {
+  Subclass* s = New<Subclass>();
+  RefCountedPtr<BaseClass> b(s);
+  EXPECT_EQ(b, s);
+}
+
+void FunctionTakingBaseClass(RefCountedPtr<BaseClass> p) {
+  p.reset();  // To appease clang-tidy.
+}
+
+TEST(RefCountedPtr, CanPassSubclassToFunctionExpectingBaseClass) {
+  RefCountedPtr<Subclass> p = MakeRefCounted<Subclass>();
+  FunctionTakingBaseClass(p);
+}
+
+void FunctionTakingSubclass(RefCountedPtr<Subclass> p) {
+  p.reset();  // To appease clang-tidy.
+}
+
+TEST(RefCountedPtr, CanPassSubclassToFunctionExpectingSubclass) {
+  RefCountedPtr<Subclass> p = MakeRefCounted<Subclass>();
+  FunctionTakingSubclass(p);
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc_core

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

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'