You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2018/06/06 20:22:09 UTC
[trafficserver] branch 8.0.x updated (150a0b4 -> eb299ca)
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a change to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.
from 150a0b4 example: Cleans up spelling
new 302d5a8 In the non-transparent case handle EADDRNOTAVAIL as a normal connect fail.
new 7144069 tsconfig/IntrusivePtr refresh - updated for eleventy and better conformance with std::shared_ptr.
new eb299ca Clang-Analyzer: Fix false positive in IntrusivePtr. Related to 40f01aa0c4fb2ab72459dcc16d63989f40bb8b9b.
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CMakeLists.txt | 1 +
lib/ts/Makefile.am | 1 +
lib/ts/unit-tests/test_IntrusivePtr.cc | 134 ++++++++++
lib/tsconfig/Errata.cc | 15 +-
lib/tsconfig/IntrusivePtr.h | 476 ++++++++++++++-------------------
lib/tsconfig/TsValue.cc | 12 +-
lib/tsconfig/TsValue.h | 4 +-
lib/wccp/WccpEndPoint.cc | 4 +-
proxy/http/HttpSM.cc | 14 +-
9 files changed, 371 insertions(+), 290 deletions(-)
create mode 100644 lib/ts/unit-tests/test_IntrusivePtr.cc
--
To stop receiving notification emails like this one, please contact
bcall@apache.org.
[trafficserver] 01/03: In the non-transparent case handle
EADDRNOTAVAIL as a normal connect fail.
Posted by bc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 302d5a8f08a1ed66446acec0de84577008d2cf3e
Author: Susan Hinrichs <sh...@apache.org>
AuthorDate: Wed May 30 15:29:25 2018 -0500
In the non-transparent case handle EADDRNOTAVAIL as a normal connect fail.
(cherry picked from commit 5847c804b7fe374aeff27ccee6e68bf7afb5ca47)
---
proxy/http/HttpSM.cc | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index ade8f1f..918a2b0 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1764,14 +1764,14 @@ HttpSM::state_http_server_open(int event, void *data)
// save the errno from the connect fail for future use (passed as negative value, flip back)
t_state.current.server->set_connect_fail(event == NET_EVENT_OPEN_FAILED ? -reinterpret_cast<intptr_t>(data) : ECONNABORTED);
- /* If we get this error, then we simply can't bind to the 4-tuple to make the connection. There's no hope of
- retries succeeding in the near future. The best option is to just shut down the connection without further
- comment. The only known cause for this is outbound transparency combined with use client target address / source
- port, as noted in TS-1424. If the keep alives desync the current connection can be attempting to rebind the 4
- tuple simultaneously with the shut down of an existing connection. Dropping the client side will cause it to pick
- a new source port and recover from this issue.
+ /* If we get this error in transparent mode, then we simply can't bind to the 4-tuple to make the connection. There's no hope
+ of retries succeeding in the near future. The best option is to just shut down the connection without further comment. The
+ only known cause for this is outbound transparency combined with use client target address / source port, as noted in
+ TS-1424. If the keep alives desync the current connection can be attempting to rebind the 4 tuple simultaneously with the
+ shut down of an existing connection. Dropping the client side will cause it to pick a new source port and recover from this
+ issue.
*/
- if (EADDRNOTAVAIL == t_state.current.server->connect_result) {
+ if (EADDRNOTAVAIL == t_state.current.server->connect_result && t_state.client_info.is_transparent) {
if (is_debug_tag_set("http_tproxy")) {
ip_port_text_buffer ip_c, ip_s;
Debug("http_tproxy", "Force close of client connect (%s->%s) due to EADDRNOTAVAIL [%" PRId64 "]",
--
To stop receiving notification emails like this one, please contact
bcall@apache.org.
[trafficserver] 02/03: tsconfig/IntrusivePtr refresh - updated for
eleventy and better conformance with std::shared_ptr.
Posted by bc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 71440695e0ab638389afeed48c125521e513232e
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Wed May 30 12:52:50 2018 -0500
tsconfig/IntrusivePtr refresh - updated for eleventy and better conformance with std::shared_ptr.
(cherry picked from commit 40f01aa0c4fb2ab72459dcc16d63989f40bb8b9b)
---
CMakeLists.txt | 1 +
lib/ts/Makefile.am | 1 +
lib/ts/unit-tests/test_IntrusivePtr.cc | 134 ++++++++++
lib/tsconfig/Errata.cc | 15 +-
lib/tsconfig/IntrusivePtr.h | 464 +++++++++++++++------------------
lib/tsconfig/TsValue.cc | 12 +-
lib/tsconfig/TsValue.h | 4 +-
lib/wccp/WccpEndPoint.cc | 4 +-
8 files changed, 364 insertions(+), 271 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1191842..edcf8d0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1309,6 +1309,7 @@ add_executable(test_tslib
lib/ts/unit-tests/test_BufferWriter.cc
lib/ts/unit-tests/test_BufferWriterFormat.cc
lib/ts/unit-tests/test_ink_inet.cc
+ lib/ts/unit-tests/test_IntrusivePtr.cc
lib/ts/unit-tests/test_IpMap.cc
lib/ts/unit-tests/test_layout.cc
lib/ts/unit-tests/test_MemArena.cc
diff --git a/lib/ts/Makefile.am b/lib/ts/Makefile.am
index fead563..f87b7b1 100644
--- a/lib/ts/Makefile.am
+++ b/lib/ts/Makefile.am
@@ -267,6 +267,7 @@ test_tslib_SOURCES = \
unit-tests/test_BufferWriter.cc \
unit-tests/test_BufferWriterFormat.cc \
unit-tests/test_ink_inet.cc \
+ unit-tests/test_IntrusivePtr.cc \
unit-tests/test_IpMap.cc \
unit-tests/test_layout.cc \
unit-tests/test_MemSpan.cc \
diff --git a/lib/ts/unit-tests/test_IntrusivePtr.cc b/lib/ts/unit-tests/test_IntrusivePtr.cc
new file mode 100644
index 0000000..79c9350
--- /dev/null
+++ b/lib/ts/unit-tests/test_IntrusivePtr.cc
@@ -0,0 +1,134 @@
+/** @file
+
+ IntrusivePtr tests.
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+#include <string>
+#include <sstream>
+#include <tsconfig/IntrusivePtr.h>
+#include <catch.hpp>
+
+struct Thing : public ts::IntrusivePtrCounter {
+ Thing() { ++_count; }
+ ~Thing() { --_count; }
+ std::string _name;
+ static int _count; // instance count.
+};
+
+struct Stuff : public Thing {
+ int _value{0};
+};
+
+struct Obscure : private ts::IntrusivePtrCounter {
+ std::string _text;
+
+ friend class ts::IntrusivePtr<Obscure>;
+};
+
+struct Atomic : ts::IntrusivePtrAtomicCounter {
+ int _q{0};
+};
+
+int Thing::_count{0};
+
+TEST_CASE("IntrusivePtr", "[libts][IntrusivePtr]")
+{
+ using Ptr = ts::IntrusivePtr<Thing>;
+
+ Ptr p1{new Thing};
+ REQUIRE(p1.use_count() == 1);
+ REQUIRE(Thing::_count == 1);
+ p1.reset(nullptr);
+ REQUIRE(Thing::_count == 0);
+
+ p1.reset(new Thing);
+ Ptr p2 = p1;
+ REQUIRE(Thing::_count == 1);
+ REQUIRE(p1.use_count() == p2.use_count());
+ REQUIRE(p2.use_count() == 2);
+
+ Ptr p3{new Thing};
+ REQUIRE(Thing::_count == 2);
+ p1 = p3;
+ p2 = p3;
+ REQUIRE(Thing::_count == 1);
+
+ Ptr p4;
+
+ REQUIRE(static_cast<bool>(p4) == false);
+ REQUIRE(!p4 == true);
+
+ REQUIRE(static_cast<bool>(p3) == true);
+ REQUIRE(!p3 == false);
+
+ // This is a compile check to make sure IntrusivePtr can be used with private inheritance
+ // of ts::InstrusivePtrCounter if IntrusivePtr is declared a friend.
+ ts::IntrusivePtr<Obscure> op{new Obscure};
+ op->_text.assign("Text");
+}
+
+// Cross type tests.
+TEST_CASE("IntrusivePtr CrossType", "[libts][IntrusivePtr]")
+{
+ using ThingPtr = ts::IntrusivePtr<Thing>;
+ using StuffPtr = ts::IntrusivePtr<Stuff>;
+
+ ThingPtr tp1{new Stuff};
+ StuffPtr sp1{new Stuff};
+
+ ThingPtr tp2{sp1};
+ REQUIRE(tp2.get() == sp1.get());
+ REQUIRE(Thing::_count == 2);
+ REQUIRE(tp2.use_count() == 2);
+ tp2 = sp1; // should be a no-op, verify it compiles.
+ REQUIRE(tp2.get() == sp1.get());
+ REQUIRE(Thing::_count == 2);
+ REQUIRE(sp1.use_count() == 2);
+ sp1 = ts::ptr_cast<Stuff>(tp1); // move assign
+ REQUIRE(sp1.get() == tp1.get());
+ REQUIRE(sp1.use_count() == 2);
+ REQUIRE(tp2.use_count() == 1);
+ tp1 = ts::ptr_cast<Stuff>(tp2); // cross type move assign
+ REQUIRE(sp1.use_count() == 1);
+ REQUIRE(tp1.get() == tp2.get());
+ REQUIRE(tp1.use_count() == 2);
+ sp1 = ts::ptr_cast<Stuff>(tp1);
+ REQUIRE(Thing::_count == 1);
+ REQUIRE(sp1.use_count() == 3);
+ tp1 = tp2; // same object assign check.
+ {
+ StuffPtr sp2{sp1};
+ REQUIRE(sp1.use_count() == 4);
+ }
+ sp1.reset();
+ tp1 = std::move(tp2); // should clear tp2
+ tp1.reset();
+ REQUIRE(Thing::_count == 0);
+}
+
+TEST_CASE("IntrusiveAtomicPtr", "[libts][IntrusivePtr]")
+{
+ using Ptr = ts::IntrusivePtr<Atomic>;
+
+ Ptr p1{new Atomic};
+ REQUIRE(p1.use_count() == 1);
+ p1.reset(nullptr);
+}
diff --git a/lib/tsconfig/Errata.cc b/lib/tsconfig/Errata.cc
index 14ab513..f91ca7f 100644
--- a/lib/tsconfig/Errata.cc
+++ b/lib/tsconfig/Errata.cc
@@ -69,9 +69,8 @@ inline Errata::Errata(ImpPtr const& ptr)
Errata::Data::~Data() {
if (m_log_on_delete) {
- Errata tmp(this); // because client API requires a wrapper.
- for ( auto& f : Sink_List ) { (*f)(tmp);
-}
+ Errata tmp(ImpPtr(this)); // because client API requires a wrapper.
+ for ( auto& f : Sink_List ) { (*f)(tmp); }
tmp.m_data.release(); // don't delete this again.
}
}
@@ -102,11 +101,11 @@ Errata::~Errata() {
Errata::Data*
Errata::pre_write() {
if (m_data) {
- if (m_data.useCount() > 1) {
- m_data = new Data(*m_data); // clone current data
+ if (m_data.use_count() > 1) {
+ m_data.reset(new Data(*m_data)); // clone current data
}
} else { // create new data
- m_data = new Data;
+ m_data.reset(new Data);
}
return m_data.get();
}
@@ -114,7 +113,7 @@ Errata::pre_write() {
// Just create an instance if needed.
Errata::Data const*
Errata::instance() {
- if (!m_data) { m_data = new Data;
+ if (!m_data) { m_data.reset(new Data);
}
return m_data.get();
}
@@ -140,7 +139,7 @@ Errata::operator=(self const& that) {
Errata&
Errata::operator = (Message const& msg) {
// Avoid copy on write in the case where we discard.
- if (!m_data || m_data.useCount() > 1) {
+ if (!m_data || m_data.use_count() > 1) {
this->clear();
this->push(msg);
} else {
diff --git a/lib/tsconfig/IntrusivePtr.h b/lib/tsconfig/IntrusivePtr.h
index 1db5054..93be87b 100644
--- a/lib/tsconfig/IntrusivePtr.h
+++ b/lib/tsconfig/IntrusivePtr.h
@@ -1,193 +1,184 @@
-#if !defined(TS_INTRUSIVE_PTR_HEADER)
-#define TS_INTRUSIVE_PTR_HEADER
-
/** @file
- This is a simple shared pointer class for restricted use. It is not a
- completely general class. The most significant missing feature is the
- lack of thread safety. For its intended use, this is acceptable and
- provides a performance improvement. However, it does restrict how the
- class may be used.
+ This is an intrusive shared pointer class designed for internal use in various containers inside
+ Traffic Server. It is not dependent on TS and could be used elsewhere. The design tries to
+ follow that of @c std::shared_ptr as it performs a similar function. The key difference is
+ this shared pointer requires the reference count to be in the target class. This is done by
+ inheriting a class containing the counter. This has the benefits
+
+ - improved locality for instances of the class and the reference count
+ - the ability to reliably construct shared pointers from raw pointers.
+ - lower overhead (a single reference counter).
- This style of shared pointer also requires explicit support from the
- target class, which must provide an internal reference counter.
+ The requirement of modifying the target class limits the generality of this class but it is
+ still quite useful in specific cases (particularly containers and their internal node classes).
@section license License
- Licensed to the Apache Software Foundation (ASF) under one
- or more contributor license agreements. See the NOTICE file
- distributed with this work for additional information
- regarding copyright ownership. The ASF licenses this file
- to you under the Apache License, Version 2.0 (the
- "License"); you may not use this file except in compliance
- with the License. You may obtain a copy of the License at
+ Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ agreements. See the NOTICE file distributed with this work for additional information regarding
+ copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with the License. You may
+ obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
+ Unless required by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ express or implied. See the License for the specific language governing permissions and
limitations under the License.
*/
+#pragma once
+
#include <sys/types.h>
+#include <type_traits>
#include <cassert>
#include <functional>
+#include <atomic>
namespace ts
{
-class IntrusivePtrCounter;
-
-/** This class exists solely to be declared a friend of @c IntrusivePtrCounter.
- @internal This is done because we can't declare the template a
- friend, so rather than burden the client with the declaration we
- do it here. It provides a single method that allows the smart pointer
- to get access to the protected reference count.
-
- */
-class IntrusivePtrBase
-{
-public:
- /// Type used for reference counter.
- typedef long Counter;
+template < typename T > class IntrusivePtr; // forward declare
-protected:
- Counter *getCounter(IntrusivePtrCounter *c ///< Cast object with reference counter.
- ) const;
-};
-/* ----------------------------------------------------------------------- */
/* ----------------------------------------------------------------------- */
/** Reference counter mixin.
- To add support for @c IntrusivePtr to class @a T, it
- should inherit from @c IntrusivePtrCounter<T> in order to
-
- - provide a reference count member
- - force the reference count to initialize to zero
- - define the add and release global functions required by @c IntrusivePtr
-
- In general this class should be inherited publicly. This will
- provide methods which mimic the @c Boost.shared_ptr interface ( @c
- unique() , @c use_count() ).
-
- If this class is not inherited publically or the destructor is
- non-public then the host class (@a T) must declare this class ( @c
- reference_counter<T> ) as a friend.
+ To add support for @c IntrusivePtr to class @a T, it must publicly inherit
+ @c IntrusivePtrCounter. Doing so
- @internal Due to changes in the C++ standard and design decisions
- in gcc, it is no longer possible to declare a template parameter
- as a friend class. (Basically, you can't use a typedef in a
- friend declaration and gcc treats template parameters as
- typedefs).
+ - provides a reference count member
+ - forces the reference count to initialize to zero
+ - prevents reference count copying on copy construction or assignment
+ - provides access to the reference counter from the shared pointer
+ - provides the `use_count` method to check the reference count.
- @note You can use this with insulated (by name only) classes. The
- important thing is to make sure that any such class that uses @c
- IntrusivePtr has all of its constructors and destructors declared
- in the header and defined in the implementation translation
- unit. If the compiler generates any of those, it will not compile
- due to missing functions or methods
+ @note You can use this with insulated (by name only) classes. The important thing is to make
+ sure that any such class that uses @c IntrusivePtr has all of its constructors and destructors
+ declared in the header and defined in the implementation translation unit. If the compiler
+ generates any of those, it will not compile due to missing functions or methods
*/
class IntrusivePtrCounter
{
- friend class IntrusivePtrBase;
+ template < typename T > friend class IntrusivePtr;
+ using self_type = IntrusivePtrCounter;
public:
/** Copy constructor.
- @internal We have to define this to explicitly _not_ copy the
- reference count. Otherwise any client that uses a default copy
- constructor will _copy the ref count into the new object_. That
- way lies madness.
+ @internal We have to define this to explicitly _not_ copy the reference count. Otherwise any
+ client that uses a default copy constructor will _copy the ref count into the new object_.
+ That way lies madness.
*/
-
- IntrusivePtrCounter(IntrusivePtrCounter const & ///< Source object.
- );
+ IntrusivePtrCounter(self_type const & that);
+ /// No move constructor - that won't work for an object that is the target of a shared pointer.
+ IntrusivePtrCounter(self_type && that) = delete;
/** Assignment operator.
- @internal We need this for the same reason as the copy
- constructor. The reference counter must not participate in
- assignment.
+ @internal We need this for the same reason as the copy constructor. The reference count must
+ not be copied.
*/
- IntrusivePtrCounter &operator=(IntrusivePtrCounter const &);
+ self_type &operator=(self_type const &that);
+ /// No move assignment - that won't work for an object that is the target of a shared pointer.
+ self_type &operator=(self_type &&that) = delete;
protected:
- IntrusivePtrBase::Counter m_intrusive_pointer_reference_count;
+ /// The reference counter type is signed because detecting underflow is more important than having
+ /// more than 2G references.
+ using intrusive_ptr_reference_counter_type = long int;
+ /// The reference counter.
+ intrusive_ptr_reference_counter_type m_intrusive_pointer_reference_count{0};
/// Default constructor (0 init counter).
/// @internal Only subclasses can access this.
IntrusivePtrCounter();
};
+
+/** Atomic reference counting mixin.
+ */
+class IntrusivePtrAtomicCounter
+{
+ template < typename T > friend class IntrusivePtr;
+
+ using self_type = IntrusivePtrAtomicCounter;
+
+public:
+ IntrusivePtrAtomicCounter(self_type const & that);
+ /// No move constructor - that won't work for an object that is the target of a shared pointer.
+ IntrusivePtrAtomicCounter(self_type && that) = delete;
+
+ self_type &operator=(self_type const &that);
+ /// No move assignment - that won't work for an object that is the target of a shared pointer.
+ self_type &operator=(self_type &&that) = delete;
+
+protected:
+ /// The reference counter type is signed because detecting underflow is more important than having
+ /// more than 2G references.
+ using intrusive_ptr_reference_counter_type = std::atomic<long int>;
+ /// The reference counter.
+ intrusive_ptr_reference_counter_type m_intrusive_pointer_reference_count{0};
+ /// Default constructor (0 init counter).
+ /// @internal Only subclasses can access this.
+ IntrusivePtrAtomicCounter();
+};
+
/* ----------------------------------------------------------------------- */
-/* ----------------------------------------------------------------------- */
-/** Shared pointer.
+/** Intrusive shared pointer.
- This is a reference counted smart pointer. A single object is jointly
- ownded by a set of pointers. When the last of the pointers is destructed
- the target object is also destructed.
+ This is a reference counted smart pointer. A single object is jointly ownded by a set of
+ pointers. When the last of the pointers is destructed the target object is also destructed.
- The smart pointer actions can be changed through class specific policy
- by specializing the @c IntrusivePtrPolicy template class.
+ The smart pointer actions can be changed through class specific policy by specializing the @c
+ IntrusivePtrPolicy template class.
*/
-template <typename T> class IntrusivePtr : private IntrusivePtrBase
+template <typename T> class IntrusivePtr
{
private: /* don't pollute client with these typedefs */
- typedef IntrusivePtrBase super; ///< Parent type.
- typedef IntrusivePtr self; ///< Self reference type.
-
+ using self_type = IntrusivePtr; ///< Self reference type.
+ template < typename U > friend class IntrusivePtr; /// Make friends wih our clones.
public:
- /// Promote type for reference counter.
- typedef super::Counter Counter;
+ /// The underlying (integral) type of the reference counter.
+ using count_type = long int;
- /// Default constructor (0 initialized).
+ /// Default constructor (nullptr initialized).
IntrusivePtr();
/// Construct from instance.
/// The instance becomes referenced and owned by the pointer.
- IntrusivePtr(T *obj);
+ explicit IntrusivePtr(T *obj);
/// Destructor.
~IntrusivePtr();
- /// Copy constructor.
- IntrusivePtr(const self &src);
- /// Self assignement.
- self &operator=(const self &src);
- /** Assign from instance.
- The instance becomes referenced and owned by the pointer.
- The reference to the current object is dropped.
- */
- self &operator=(T *obj ///< Target instance.
- );
+ /// Copy constructor
+ IntrusivePtr(self_type const& that);
+ /// Move constructor.
+ IntrusivePtr(self_type && that);
+ /// Self assignment.
+ self_type& operator=(self_type const& that);
+ /// Move assignment.
+ self_type& operator=(self_type && that);
/** Assign from instance.
The instance becomes referenced and owned by the pointer.
The reference to the current object is dropped.
- @note A synonym for @c operator= for compatibility.
*/
- self &assign(T *obj ///< Target instance.
- );
+ void reset(T *obj = nullptr);
- /** Assign from instance.
- The instance becomes referenced and owned by the pointer.
- The reference to the current object is dropped.
- */
- void reset(T *obj);
/** Clear reference without cleanup.
- This unsets this smart pointer and decrements the reference
- count, but does @b not perform any finalization on the
- target object. This can easily lead to memory leaks and
- in some sense vitiates the point of this class, but it is
- occasionally the right thing to do. Use with caution.
+ This unsets this smart pointer and decrements the reference count, but does @b not perform any
+ finalization on the target object. This can easily lead to memory leaks and in some sense
+ vitiates the point of this class, but it is occasionally the right thing to do. Use with
+ caution.
@return @c true if there are no references upon return,
@c false if the reference count is not zero.
*/
- bool release();
+ T* release();
- /// Test if the pointer is zero (@c NULL).
- bool isNull() const;
+ /// Test for not @c nullptr
+ explicit operator bool() const;
/// Member dereference.
T *operator->() const;
@@ -198,53 +189,41 @@ public:
/** User conversion to raw pointer.
- @internal allow implicit conversion to the underlying
- pointer. This allows for the form "if (handle)" and is not
- particularly dangerous (as it would be for a scope_ptr or
- std::shared_ptr) because the counter is carried with the object and
- so can't get lost or duplicated.
+ @internal allow implicit conversion to the underlying pointer which is one of the advantages
+ of intrusive pointers because the reference count doesn't get lost.
*/
operator T *() const;
/** Cross type construction.
- This succeeds if an @a X* can be implicitly converted to a @a T*.
+ This succeeds if an @a U* can be implicitly converted to a @a T*.
*/
- template <typename X ///< Foreign pointer type.
- >
- IntrusivePtr(IntrusivePtr<X> const &that ///< Foreign pointer.
- );
+ template <typename U>
+ IntrusivePtr(IntrusivePtr<U> const &that);
+
+ template <typename U>
+ IntrusivePtr(IntrusivePtr<U> && that);
/** Cross type assignment.
- This succeeds if an @a X* can be implicitily converted to a @a T*.
+ This succeeds if an @a U* can be implicitily converted to a @a T*.
*/
- template <typename X ///< Foreign pointer type.
- >
- self &operator=(IntrusivePtr<X> const &that ///< Foreign pointer.
- );
-
- /// Check for multiple references.
- /// @return @c true if more than one smart pointer references the object,
- /// @c false otherwise.
- bool isShared() const;
- /// Check for a single reference (@c std::shared_ptr compatibility)
- /// @return @c true if this object is not shared.
- bool unique() const;
+ template <typename U>
+ self_type &operator=(IntrusivePtr<U> const &that);
+
+ template <typename U>
+ self_type &operator=(IntrusivePtr<U> && that);
+
/// Reference count.
/// @return Number of references.
- Counter useCount() const;
+ count_type use_count() const;
private:
- T *m_obj; ///< Pointer to object.
+ T *m_obj{nullptr}; ///< Pointer to object.
/// Reference @a obj.
- void set(T *obj ///< Target object.
- );
+ void set(T *obj);
/// Drop the current reference.
void unset();
-
- /// Get a pointer to the reference counter of the target object.
- Counter *getCounter() const;
};
/** Pointer dynamic cast.
@@ -260,11 +239,9 @@ private:
the_b = dynamic_ptr_cast<B>(really_b);
@endcode
*/
-template <typename T, ///< Target type.
- typename X ///< Source type.
- >
+template <typename T, typename U>
IntrusivePtr<T>
-dynamic_ptr_cast(IntrusivePtr<X> const &src ///< Source pointer.
+dynamic_ptr_cast(IntrusivePtr<U> const &src ///< Source pointer.
)
{
return IntrusivePtr<T>(dynamic_cast<T *>(src.get()));
@@ -285,10 +262,10 @@ dynamic_ptr_cast(IntrusivePtr<X> const &src ///< Source pointer.
@endcode
*/
template <typename T, ///< Target type.
- typename X ///< Source type.
+ typename U ///< Source type.
>
IntrusivePtr<T>
-ptr_cast(IntrusivePtr<X> const &src ///< Source pointer.
+ptr_cast(IntrusivePtr<U> const &src ///< Source pointer.
)
{
return IntrusivePtr<T>(static_cast<T *>(src.get()));
@@ -357,27 +334,29 @@ typedef IntrusivePtrPolicy<IntrusivePtrDefaultPolicyTag> IntrusivePtrDefaultPoli
/* ----------------------------------------------------------------------- */
/* ----------------------------------------------------------------------- */
/* Inline Methods */
-inline IntrusivePtrCounter::IntrusivePtrCounter() : m_intrusive_pointer_reference_count(0)
-{
-}
+inline IntrusivePtrCounter::IntrusivePtrCounter() {}
-inline IntrusivePtrCounter::IntrusivePtrCounter(IntrusivePtrCounter const &) : m_intrusive_pointer_reference_count(0)
-{
-}
+inline IntrusivePtrCounter::IntrusivePtrCounter(self_type const &) {}
inline IntrusivePtrCounter &
-IntrusivePtrCounter::operator=(IntrusivePtrCounter const &)
+IntrusivePtrCounter::operator=(self_type const &)
{
return *this;
}
-inline IntrusivePtrBase::Counter *
-IntrusivePtrBase::getCounter(IntrusivePtrCounter *c) const
+inline IntrusivePtrAtomicCounter::IntrusivePtrAtomicCounter() {}
+
+inline IntrusivePtrAtomicCounter::IntrusivePtrAtomicCounter(self_type const &) {}
+
+inline IntrusivePtrAtomicCounter &
+IntrusivePtrAtomicCounter::operator=(self_type const &)
{
- return &(c->m_intrusive_pointer_reference_count);
+ return *this;
}
+
/* ----------------------------------------------------------------------- */
/* ----------------------------------------------------------------------- */
+
template <typename T>
void
IntrusivePtrPolicy<T>::dereferenceCheck(T *)
@@ -399,9 +378,7 @@ IntrusivePtrPolicy<T>::Order::operator()(IntrusivePtr<T> const &lhs, IntrusivePt
}
/* ----------------------------------------------------------------------- */
/* ----------------------------------------------------------------------- */
-template <typename T> IntrusivePtr<T>::IntrusivePtr() : m_obj(nullptr)
-{
-}
+template <typename T> IntrusivePtr<T>::IntrusivePtr() {}
template <typename T> IntrusivePtr<T>::IntrusivePtr(T *obj)
{
@@ -413,50 +390,81 @@ template <typename T> IntrusivePtr<T>::~IntrusivePtr()
this->unset();
}
-template <typename T> IntrusivePtr<T>::IntrusivePtr(const self &that)
+template < typename T >
+IntrusivePtr<T>::IntrusivePtr(self_type const &that)
{
this->set(that.m_obj);
}
template <typename T>
-template <typename X>
-IntrusivePtr<T>::IntrusivePtr(IntrusivePtr<X> const &that ///< Foreign pointer.
- )
- : super(that.get())
+template <typename U>
+IntrusivePtr<T>::IntrusivePtr(IntrusivePtr<U> const &that)
{
+ static_assert(std::is_convertible<U*, T*>::value, "The argument type is not implicitly convertible to the return type.");
+ this->set(that.m_obj);
+}
+
+template < typename T >
+IntrusivePtr<T>::IntrusivePtr(self_type && that)
+{
+ std::swap<T*>(m_obj, that.m_obj);
}
template <typename T>
+template <typename U>
+IntrusivePtr<T>::IntrusivePtr(IntrusivePtr<U> && that)
+{
+ static_assert(std::is_convertible<U*, T*>::value, "The argument type is not implicitly convertible to the return type.");
+ std::swap<T*>(m_obj, that.get());
+}
+
+template < typename T >
IntrusivePtr<T> &
-IntrusivePtr<T>::operator=(const self &that)
+IntrusivePtr<T>::operator=(self_type const& that)
{
- this->reset(that.m_obj);
+ this->reset(that.get());
return *this;
}
+
template <typename T>
-template <typename X>
+template <typename U>
IntrusivePtr<T> &
-IntrusivePtr<T>::operator=(IntrusivePtr<X> const &that ///< Foreign pointer.
- )
+IntrusivePtr<T>::operator=(IntrusivePtr<U> const& that)
{
+ static_assert(std::is_convertible<U*, T*>::value, "The argument type is not implicitly convertible to the return type.");
this->reset(that.get());
return *this;
}
-template <typename T>
+template < typename T >
IntrusivePtr<T> &
-IntrusivePtr<T>::operator=(T *obj)
+IntrusivePtr<T>::operator=(self_type && that)
{
- this->reset(obj);
+ if (m_obj != that.m_obj) {
+ this->unset();
+ m_obj = that.m_obj;
+ that.m_obj = nullptr;
+ } else {
+ that.unset();
+ }
return *this;
}
template <typename T>
+template <typename U>
IntrusivePtr<T> &
-IntrusivePtr<T>::assign(T *obj)
+IntrusivePtr<T>::operator=(IntrusivePtr<U> && that)
{
- return *this = obj;
+ static_assert(std::is_convertible<U*, T*>::value, "The argument type is not implicitly convertible to the return type.");
+ if (m_obj != that.m_obj) {
+ this->unset();
+ m_obj = that.m_obj;
+ that.m_obj = nullptr;
+ } else {
+ that.unset();
+ }
+ return *this;
}
template <typename T> T *IntrusivePtr<T>::operator->() const
@@ -479,21 +487,14 @@ IntrusivePtr<T>::get() const
return m_obj;
}
-template <typename T>
-typename IntrusivePtr<T>::Counter *
-IntrusivePtr<T>::getCounter() const
-{
- return super::getCounter(static_cast<IntrusivePtrCounter *>(m_obj));
-}
-
/* The Set/Unset methods are the basic implementation of our
* reference counting. The Reset method is the standard way
* of invoking the pair, although splitting them allows some
* additional efficiency in certain situations.
*/
-/* set and unset are two half operations that don't do checks.
- It is the callers responsibility to do that.
+/* @c set and @c unset are two half operations that don't do checks.
+ It is the caller's responsibility to do that.
*/
template <typename T>
@@ -506,16 +507,16 @@ IntrusivePtr<T>::unset()
* super class. We call the super class method to get a raw pointer
* to the counter variable.
*/
- Counter *cp = this->getCounter();
+ auto &cp = m_obj->m_intrusive_pointer_reference_count;
/* If you hit this assert you've got a cycle of objects that
reference each other. A delete in the cycle will eventually
result in one of the objects getting deleted twice, which is
what this assert indicates.
*/
- assert(*cp);
+ assert(cp);
- if (0 == --*cp) {
+ if (0 == --cp) {
IntrusivePtrPolicy<T>::finalize(m_obj);
}
m_obj = nullptr;
@@ -528,7 +529,7 @@ IntrusivePtr<T>::set(T *obj)
{
m_obj = obj; /* update to new object */
if (nullptr != m_obj) /* if a real object, bump the ref count */
- ++(*(this->getCounter()));
+ ++(m_obj->m_intrusive_pointer_reference_count);
}
template <typename T>
@@ -542,28 +543,25 @@ IntrusivePtr<T>::reset(T *obj)
}
template <typename T>
-bool
+T*
IntrusivePtr<T>::release()
{
- bool zret = true;
+ T* zret = m_obj;
if (m_obj) {
- Counter *cp = this->getCounter();
- zret = *cp <= 1;
+ auto &cp = m_obj->m_intrusive_pointer_reference_count;
// If the client is using this method, they're doing something funky
// so be extra careful with the reference count.
- if (*cp > 0)
- --*cp;
+ if (cp > 0)
+ --cp;
m_obj = nullptr;
}
return zret;
}
-/* Simple method to check for invalid pointer */
template <typename T>
-bool
-IntrusivePtr<T>::isNull() const
+IntrusivePtr<T>::operator bool() const
{
- return 0 == m_obj;
+ return m_obj != nullptr;
}
/* Pointer comparison */
@@ -588,63 +586,17 @@ operator<(IntrusivePtr<T> const &lhs, IntrusivePtr<T> const &rhs)
return lhs.get() < rhs.get();
}
-template <typename T>
-bool
-operator==(IntrusivePtr<T> const &lhs, int rhs)
-{
- assert(0 == rhs);
- return lhs.get() == 0;
-}
-
-template <typename T>
-bool
-operator==(int lhs, IntrusivePtr<T> const &rhs)
-{
- assert(0 == lhs);
- return rhs.get() == nullptr;
-}
-
-template <typename T>
-bool
-operator!=(int lhs, IntrusivePtr<T> const &rhs)
-{
- return !(lhs == rhs);
-}
-
-template <typename T>
-bool
-operator!=(IntrusivePtr<T> const &lhs, int rhs)
-{
- return !(lhs == rhs);
-}
-
template <typename T> IntrusivePtr<T>::operator T *() const
{
return m_obj;
}
template <typename T>
-bool
-IntrusivePtr<T>::isShared() const
+typename IntrusivePtr<T>::count_type
+IntrusivePtr<T>::use_count() const
{
- return m_obj && *(this->getCounter()) > 1;
+ return m_obj ? count_type{m_obj->m_intrusive_pointer_reference_count} : count_type{0};
}
-
-template <typename T>
-bool
-IntrusivePtr<T>::unique() const
-{
- return 0 == m_obj || *(this->getCounter()) <= 1;
-}
-
-template <typename T>
-typename IntrusivePtr<T>::Counter
-IntrusivePtr<T>::useCount() const
-{
- return m_obj ? *(this->getCounter()) : 0;
-}
-/* ----------------------------------------------------------------------- */
/* ----------------------------------------------------------------------- */
-} // namespace ats
/* ----------------------------------------------------------------------- */
-#endif // TS_INTRUSIVE_PTR_HEADER
+} // namespace ts
diff --git a/lib/tsconfig/TsValue.cc b/lib/tsconfig/TsValue.cc
index 0aea522..de0298f 100644
--- a/lib/tsconfig/TsValue.cc
+++ b/lib/tsconfig/TsValue.cc
@@ -62,8 +62,13 @@ detail::ValueTableImpl::~ValueTableImpl() {
// ---------------------------------------------------------------------------
detail::ValueTable::ImplType*
detail::ValueTable::instance() {
- if (! _ptr) { _ptr.reset(new ImplType);
-}
+ // Stupid workaround for clang analyzer false positive. The new instance isn't leaked because
+ // it's stored in a shared ptr type.
+#if !defined(__clang_analyzer__)
+ if (! _ptr) { _ptr.reset(new ImplType); }
+#else
+ assert(_ptr.get() != nullptr);
+#endif
return _ptr.get();
}
@@ -229,7 +234,8 @@ Value::makePath(Path const& path, ConstBuffer const& name) {
Path& Path::reset() {
if (_ptr) {
// If we're sharing the instance, make a new one for us.
- if (_ptr.isShared()) { _ptr = new ImplType;
+ if (_ptr.use_count() > 1) {
+ _ptr.reset(new ImplType);
} else { // clear out the existing instance.
_ptr->_elements.clear();
}
diff --git a/lib/tsconfig/TsValue.h b/lib/tsconfig/TsValue.h
index db8fc3f..f8fc4e1 100644
--- a/lib/tsconfig/TsValue.h
+++ b/lib/tsconfig/TsValue.h
@@ -642,7 +642,7 @@ namespace detail {
inline size_t ValueTable::size() const { return _ptr ? _ptr->_values.size() : 0; }
inline Generation ValueTable::generation() const { return _ptr ? _ptr->_generation : Generation(0); }
inline ValueItem const& ValueTable::operator [] (ValueIndex idx) const { return const_cast<self*>(this)->operator [] (idx); }
- inline ValueTable& ValueTable::reset() { _ptr = nullptr; return *this; }
+ inline ValueTable& ValueTable::reset() { _ptr.reset(); return *this; }
inline ValueItem::ValueItem() : _type(VoidValue), _local_index(0), _srcLine(0), _srcColumn(0) {}
inline ValueItem::ValueItem(ValueType type) : _type(type), _local_index(0), _srcLine(0), _srcColumn(0) {}
@@ -716,7 +716,7 @@ inline Value& Value::setSource(int line, int col) {
inline Path::ImplType::ImplType() { }
inline Path::Path() { }
-inline Path::ImplType* Path::instance() { if (!_ptr) _ptr = new ImplType; return _ptr.get(); }
+inline Path::ImplType* Path::instance() { if (!_ptr) _ptr.reset(new ImplType); return _ptr.get(); }
inline Path& Path::append(ConstBuffer const& tag) { this->instance()->_elements.push_back(tag); return *this; }
inline Path& Path::append(size_t index) { this->instance()->_elements.push_back(ConstBuffer(nullptr, index)); return *this; }
inline size_t Path::count() const { return _ptr ? _ptr->_elements.size() : 0; }
diff --git a/lib/wccp/WccpEndPoint.cc b/lib/wccp/WccpEndPoint.cc
index 2815875..b69c5bc 100644
--- a/lib/wccp/WccpEndPoint.cc
+++ b/lib/wccp/WccpEndPoint.cc
@@ -1165,7 +1165,7 @@ Cache::~Cache() {}
EndPoint::ImplType *
Cache::make()
{
- m_ptr.assign(new ImplType);
+ m_ptr.reset(new ImplType);
return m_ptr.get();
}
@@ -1219,7 +1219,7 @@ Router::~Router() {}
EndPoint::ImplType *
Router::make()
{
- m_ptr.assign(new ImplType);
+ m_ptr.reset(new ImplType);
return m_ptr.get();
}
--
To stop receiving notification emails like this one, please contact
bcall@apache.org.
[trafficserver] 03/03: Clang-Analyzer: Fix false positive in
IntrusivePtr. Related to 40f01aa0c4fb2ab72459dcc16d63989f40bb8b9b.
Posted by bc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit eb299ca22e238aea6211c23059ec7fb7ff0f9e99
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Sat Jun 2 09:29:55 2018 -0500
Clang-Analyzer: Fix false positive in IntrusivePtr.
Related to 40f01aa0c4fb2ab72459dcc16d63989f40bb8b9b.
(cherry picked from commit 2183065f8f4597b4a6d78ad1995ef5a5df119b24)
---
lib/tsconfig/IntrusivePtr.h | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/lib/tsconfig/IntrusivePtr.h b/lib/tsconfig/IntrusivePtr.h
index 93be87b..93370f3 100644
--- a/lib/tsconfig/IntrusivePtr.h
+++ b/lib/tsconfig/IntrusivePtr.h
@@ -502,20 +502,8 @@ void
IntrusivePtr<T>::unset()
{
if (nullptr != m_obj) {
- /* magic: our target is required to inherit from IntrusivePtrCounter,
- * which provides a protected counter variable and access via our
- * super class. We call the super class method to get a raw pointer
- * to the counter variable.
- */
auto &cp = m_obj->m_intrusive_pointer_reference_count;
- /* If you hit this assert you've got a cycle of objects that
- reference each other. A delete in the cycle will eventually
- result in one of the objects getting deleted twice, which is
- what this assert indicates.
- */
- assert(cp);
-
if (0 == --cp) {
IntrusivePtrPolicy<T>::finalize(m_obj);
}
--
To stop receiving notification emails like this one, please contact
bcall@apache.org.