allow both old and new buffer impls to be used in the same process (to support the coverage tests)

brian-pane · brian-pane · commit cfd9d4a068e3 · 2019-03-03T19:26:01.000-08:00
Signed-off-by: Brian Pane &lt;brianp+github@brianp.net&gt;
diff --git a/source/common/buffer/buffer_impl.cc b/source/common/buffer/buffer_impl.cc
@@ -91,6 +91,7 @@ void OwnedImpl::prepend(absl::string_view data) {
 
 void OwnedImpl::prepend(Instance& data) {
   ASSERT(&data != this);
+  ASSERT(isSameBufferImpl(data));
   // See the comments in move() for why we do the static_cast.
   if (old_impl_) {
     ASSERT(dynamic_cast<LibEventInstance*>(&data) != nullptr);
@@ -295,6 +296,7 @@ void* OwnedImpl::linearize(uint32_t size) {
 
 void OwnedImpl::move(Instance& rhs) {
   ASSERT(&rhs != this);
+  ASSERT(isSameBufferImpl(rhs));
   if (old_impl_) {
     // We do the static cast here because in practice we only have one buffer implementation right
     // now and this is safe. Using the evbuffer move routines require having access to both
@@ -323,6 +325,7 @@ void OwnedImpl::move(Instance& rhs) {
 
 void OwnedImpl::move(Instance& rhs, uint64_t length) {
   ASSERT(&rhs != this);
+  ASSERT(isSameBufferImpl(rhs));
   if (old_impl_) {
     // See move() above for why we do the static cast.
     int rc = evbuffer_remove_buffer(static_cast<LibEventInstance&>(rhs).buffer().get(),
@@ -688,16 +691,16 @@ void OwnedImpl::appendSliceForTest(absl::string_view data) {
 }
 
 void OwnedImpl::useOldImpl(bool use_old_impl) {
-  static bool called_already = false;
-  if (use_old_impl != use_old_impl_) {
-    RELEASE_ASSERT(!called_already,
-                   "It is unsafe to change the buffer implementation without a restart");
-    use_old_impl_ = use_old_impl;
-  }
-  called_already = true;
+  use_old_impl_ = use_old_impl;
 }
 
-void OwnedImpl::useOldImplForTest(bool use_old_impl) { use_old_impl_ = use_old_impl; }
+bool OwnedImpl::isSameBufferImpl(const Instance& rhs) const {
+  const OwnedImpl* other = dynamic_cast<const OwnedImpl*>(&rhs);
+  if (other == nullptr) {
+    return false;
+  }
+  return usesOldImpl() == other->usesOldImpl();
+}
 
 bool OwnedImpl::use_old_impl_ = false;
 
diff --git a/source/common/buffer/buffer_impl.h b/source/common/buffer/buffer_impl.h
@@ -525,22 +525,22 @@ class OwnedImpl : public LibEventInstance {
 
   /**
    * @param use_old_impl whether to use the evbuffer-based implementation for new buffers
-   * @warning This method may be called at most once; it will terminate the program if called
-   *          twice. It should be called before any OwnedImpl objects are created. The reason
-   *          is that it is unsafe to mix and match buffers with different implementations.
-   *          The move() method, in particular, only works if the source and destination
-   *          objects are using the same destination.
+   * @warning Except for testing code, this method should be called at most once per process,
+   *          before any OwnedImpl objects are created. The reason is that it is unsafe to
+   *          mix and match buffers with different implementations. The move() method,
+   *          in particular, only works if the source and destination objects are using
+   *          the same destination.
    */
   static void useOldImpl(bool use_old_impl);
 
+private:
   /**
-   * Variant of useOldImpl that may be called repeatedly to switch between the
-   * old and new implementations (for use in test code only).
-   * @param use_old_impl whether to use the evbuffer-based implementation for new buffers.
+   * @param rhs another buffer
+   * @return whether the rhs buffer is also an instance of OwnedImpl (or a subclass) that
+   *         uses the same internal implementation as this buffer.
    */
-  static void useOldImplForTest(bool use_old_impl);
+  bool isSameBufferImpl(const Instance& rhs) const;
 
-private:
   /** Whether to use the old evbuffer implementation when constructing new OwnedImpl objects. */
   static bool use_old_impl_;
 
diff --git a/test/common/buffer/utility.h b/test/common/buffer/utility.h
@@ -22,7 +22,7 @@ enum class BufferImplementation {
 class BufferImplementationParamTest : public testing::TestWithParam<BufferImplementation> {
 protected:
   BufferImplementationParamTest() {
-    OwnedImpl::useOldImplForTest(GetParam() == BufferImplementation::Old);
+    OwnedImpl::useOldImpl(GetParam() == BufferImplementation::Old);
   }
 
   virtual ~BufferImplementationParamTest() {}

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ enum class BufferImplementation {`
`22`	`22`	`class BufferImplementationParamTest : public testing::TestWithParam<BufferImplementation> {`
`23`	`23`	`protected:`
`24`	`24`	`BufferImplementationParamTest() {`
`25`		`- OwnedImpl::useOldImplForTest(GetParam() == BufferImplementation::Old);`
	`25`	`+ OwnedImpl::useOldImpl(GetParam() == BufferImplementation::Old);`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`virtual ~BufferImplementationParamTest() {}`