Author: brane
Date: Tue Jan 8 14:29:31 2019
New Revision: 1850753
URL: http://svn.apache.org/viewvc?rev=1850753&view=rev
Log:
Keep the SVN++ global pool alive as long as there are any users.
[in subversion/bindings/cxx]
* include/svnxx/init.hpp
(detail::global_state): Renamed from detail::context.
(init): Derive directly from the global_state shared pointer.
(init::~init): Add explicit destructor (for debugging).
* src/private/init_private.hpp: Do not include svn_private_config.h.
(detail::global_state): Renamed from detail::context.
(detail::global_state::get): Don't try to translate the exception
message, because neither APR nor the localisation library are
likely to have been initialized.
* src/aprwrap/pool.hpp: Include init_private.hpp.
(apr::pool::pool): Add a constructor overload that uses the global
pool from a known global state reference.
* src/aprwrap/impl.cpp
(apr::pool::get_root_pool): Rename detail::context to detail::global_state.
* src/debug.cpp,
src/private/debug_private.hpp: New files.
* src/private/client_context_private.hpp: Include init_private.hpp.
(client::context::state): New member, keeps the global state alive as
long as the client context is viable.
(client::context::get_state): New accessor.
(client::context::get_ctx): Renamed from 'get'.
(impl::unwrap): Make the context_wrapper a final class.
* src/init.cpp:
Include debug_private.hpp and apr_pools.h instead of svn_pools.h.
(init::init): Use global_state and add pool debugging logs.
(init::~init): Implement, with pool debugging logs.
(notify_root_pool_cleanup): New; tracks the global pool's destruction.
(create_root_pool): Add pool debugging logs. Avoid leaking the root pool.
(detail::global_state): Add pool debugging logs.
* src/client_status.cpp
(client::status): use the client context's renamed get_ctx() accessor.
* tests/test_init.cpp: Rename detail::context to detail::global_state.
Added:
subversion/trunk/subversion/bindings/cxx/src/debug.cpp (with props)
subversion/trunk/subversion/bindings/cxx/src/private/debug_private.hpp
(with props)
Modified:
subversion/trunk/subversion/bindings/cxx/include/svnxx/init.hpp
subversion/trunk/subversion/bindings/cxx/src/aprwrap/impl.cpp
subversion/trunk/subversion/bindings/cxx/src/aprwrap/pool.hpp
subversion/trunk/subversion/bindings/cxx/src/client_status.cpp
subversion/trunk/subversion/bindings/cxx/src/init.cpp
subversion/trunk/subversion/bindings/cxx/src/private/client_context_private.hpp
subversion/trunk/subversion/bindings/cxx/src/private/init_private.hpp
subversion/trunk/subversion/bindings/cxx/tests/test_init.cpp
Modified: subversion/trunk/subversion/bindings/cxx/include/svnxx/init.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/include/svnxx/init.hpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/include/svnxx/init.hpp (original)
+++ subversion/trunk/subversion/bindings/cxx/include/svnxx/init.hpp Tue Jan 8
14:29:31 2019
@@ -33,7 +33,7 @@ namespace svnxx {
namespace detail {
// Forward declaration of the private API context.
-class context;
+class global_state;
} // namespace detail
/**
@@ -44,13 +44,13 @@ class context;
* create an @c init object before you can use the SVN++ API. It is
* safe to create create any number of these objects.
*/
-class init
+class init : private std::shared_ptr<detail::global_state>
{
+ using state = std::shared_ptr<detail::global_state>;
+
public:
init();
-
-private:
- std::shared_ptr<detail::context> context;
+ ~init() noexcept;
};
} // namespace svnxx
Modified: subversion/trunk/subversion/bindings/cxx/src/aprwrap/impl.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/aprwrap/impl.cpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/aprwrap/impl.cpp (original)
+++ subversion/trunk/subversion/bindings/cxx/src/aprwrap/impl.cpp Tue Jan 8
14:29:31 2019
@@ -37,8 +37,8 @@ namespace apr {
apr_pool_t* pool::get_root_pool()
{
- auto ctx = detail::context::get();
- return ctx->get_root_pool();
+ auto state = detail::global_state::get();
+ return state->get_root_pool();
}
//
Modified: subversion/trunk/subversion/bindings/cxx/src/aprwrap/pool.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/aprwrap/pool.hpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/aprwrap/pool.hpp (original)
+++ subversion/trunk/subversion/bindings/cxx/src/aprwrap/pool.hpp Tue Jan 8
14:29:31 2019
@@ -31,6 +31,8 @@
#include "svnxx/exception.hpp"
#include "svnxx/detail/noncopyable.hpp"
+#include "../private/init_private.hpp"
+
#include "svn_pools.h"
namespace apache {
@@ -101,6 +103,13 @@ public:
{}
/**
+ * Create a pool as a child of the global pool in @a state.
+ */
+ explicit pool(const detail::global_state::ptr& state)
+ : pool_ptr(svn_pool_create(state->get_root_pool()))
+ {}
+
+ /**
* Clear the pool.
*/
void clear()
Modified: subversion/trunk/subversion/bindings/cxx/src/client_status.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/client_status.cpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/client_status.cpp (original)
+++ subversion/trunk/subversion/bindings/cxx/src/client_status.cpp Tue Jan 8
14:29:31 2019
@@ -68,7 +68,7 @@ status(context& ctx_, const char* path,
svn_revnum_t result;
impl::checked_call(
- svn_client_status6(&result, ctx.get(), path, &rev,
+ svn_client_status6(&result, ctx.get_ctx(), path, &rev,
impl::convert(depth),
bool(flags & status_flags::get_all),
bool(flags & status_flags::check_out_of_date),
Added: subversion/trunk/subversion/bindings/cxx/src/debug.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/debug.cpp?rev=1850753&view=auto
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/debug.cpp (added)
+++ subversion/trunk/subversion/bindings/cxx/src/debug.cpp Tue Jan 8 14:29:31
2019
@@ -0,0 +1,37 @@
+/**
+ * @copyright
+ * ====================================================================
+ * 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.
+ * ====================================================================
+ * @endcopyright
+ */
+
+#include "private/debug_private.hpp"
+
+namespace apache {
+namespace subversion {
+namespace svnxx {
+namespace impl {
+
+const char root_pool_tag[] = "svn++ root pool";
+const char root_pool_key[] = "SVN++ 30F4B67A-CAB4-1337-F00D-DEADBEEFA78F";
+
+} // namespace impl
+} // namespace svnxx
+} // namespace subversion
+} // namespace apache
Propchange: subversion/trunk/subversion/bindings/cxx/src/debug.cpp
------------------------------------------------------------------------------
svn:eol-style = native
Modified: subversion/trunk/subversion/bindings/cxx/src/init.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/init.cpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/init.cpp (original)
+++ subversion/trunk/subversion/bindings/cxx/src/init.cpp Tue Jan 8 14:29:31
2019
@@ -24,18 +24,30 @@
#include <sstream>
#include "svnxx/exception.hpp"
+#include "private/debug_private.hpp"
#include "private/init_private.hpp"
#include <apr_general.h>
-#include "svn_pools.h"
+#include <apr_pools.h>
namespace apache {
namespace subversion {
namespace svnxx {
init::init()
- : context(detail::context::create())
-{}
+ : state(detail::global_state::create())
+{
+#ifdef SVNXX_POOL_DEBUG
+ SVN_DBG(("svn++ created init object %pp", static_cast<void*>(this)));
+#endif
+}
+
+init::~init() noexcept
+{
+#ifdef SVNXX_POOL_DEBUG
+ SVN_DBG(("svn++ destroyed init object %pp", static_cast<void*>(this)));
+#endif
+}
namespace detail {
@@ -52,6 +64,15 @@ int handle_failed_allocation(int)
throw allocation_failed_builder("svn::allocation_failed");
}
+#ifdef SVNXX_POOL_DEBUG
+apr_status_t notify_root_pool_cleanup(void* key)
+{
+ if (key == impl::root_pool_key)
+ SVN_DBG(("svn++ destroyed root pool"));
+ return APR_SUCCESS;
+}
+#endif
+
apr_pool_t* create_root_pool()
{
// Create the root pool's allocator.
@@ -68,42 +89,56 @@ apr_pool_t* create_root_pool()
throw allocation_failed_builder("svn++ creating root pool");
#if APR_POOL_DEBUG
- apr_pool_tag(root_pool, "svn++ root pool");
+ apr_pool_tag(root_pool, impl::root_pool_tag);
#endif
#if APR_HAS_THREADS
// SVN++ pools are always as thread safe as APR can make them.
apr_thread_mutex_t *mutex = nullptr;
status = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT,
root_pool);
- if (status || !mutex)
- throw allocation_failed_builder("svn++ creating allocator mutex");
- apr_allocator_mutex_set(allocator, mutex);
+ if (mutex && !status)
+ apr_allocator_mutex_set(allocator, mutex);
+ else
+ {
+#ifdef SVNXX_POOL_DEBUG
+ SVN_DBG(("svn++ could not create allocator mutex, apr_err=%d", status));
+#endif
+ apr_pool_destroy(root_pool); // Don't leak the global pool.
+ throw allocation_failed_builder("svn++ creating allocator mutex");
+ }
+#endif
+
+#ifdef SVNXX_POOL_DEBUG
+ apr_pool_cleanup_register(root_pool, impl::root_pool_key,
+ notify_root_pool_cleanup,
+ apr_pool_cleanup_null);
+ SVN_DBG(("svn++ created root pool"));
#endif
return root_pool;
}
} // anonymous namespace
-std::mutex context::guard;
-context::weak_ptr context::self;
+std::mutex global_state::guard;
+global_state::weak_ptr global_state::self;
-context::ptr context::create()
+global_state::ptr global_state::create()
{
std::unique_lock<std::mutex> lock(guard);
- auto ctx = self.lock();
- if (!ctx)
+ auto state = self.lock();
+ if (!state)
{
// Work around the private constructor: since this struct is
// defined within a class member, the the private constructor
// is accessible and std::make_shared won't complain about it.
- struct make_shared_hack : public context {};
- ctx = std::make_shared<make_shared_hack>();
- self = ctx;
+ struct global_state_builder final : public global_state {};
+ state = std::make_shared<global_state_builder>();
+ self = state;
}
- return ctx;
+ return state;
}
-context::context()
+global_state::global_state()
{
const auto status = apr_initialize();
if (status)
@@ -114,11 +149,18 @@ context::context()
<< apr_strerror(status, errbuf, sizeof(errbuf) - 1);
throw std::runtime_error(message.str());
}
+
root_pool = create_root_pool();
+#ifdef SVNXX_POOL_DEBUG
+ SVN_DBG(("svn++ created global state"));
+#endif
}
-context::~context()
+global_state::~global_state()
{
+#ifdef SVNXX_POOL_DEBUG
+ SVN_DBG(("svn++ destroyed global state"));
+#endif
std::unique_lock<std::mutex> lock(guard);
apr_terminate();
root_pool = nullptr;
Modified:
subversion/trunk/subversion/bindings/cxx/src/private/client_context_private.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/private/client_context_private.hpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
---
subversion/trunk/subversion/bindings/cxx/src/private/client_context_private.hpp
(original)
+++
subversion/trunk/subversion/bindings/cxx/src/private/client_context_private.hpp
Tue Jan 8 14:29:31 2019
@@ -26,6 +26,7 @@
#include "svnxx/client/context.hpp"
+#include "../private/init_private.hpp"
#include "../aprwrap.hpp"
#include "svn_client.h"
@@ -39,15 +40,21 @@ namespace detail {
// TODO: document this
class context
{
+ using global_state = svnxx::detail::global_state;
+
public:
context()
- : ctx(create_ctx(ctx_pool))
+ : state(global_state::get()),
+ ctx_pool(state),
+ ctx(create_ctx(ctx_pool))
{}
- svn_client_ctx_t* get() const noexcept { return ctx; };
+ const global_state::ptr& get_state() const noexcept { return state; }
const apr::pool& get_pool() const noexcept { return ctx_pool; }
+ svn_client_ctx_t* get_ctx() const noexcept { return ctx; };
private:
+ const global_state::ptr state;
apr::pool ctx_pool;
svn_client_ctx_t* const ctx;
@@ -61,7 +68,7 @@ namespace impl {
// TODO: document this
inline client::detail::context& unwrap(client::context& ctx)
{
- struct context_wrapper : public client::context
+ struct context_wrapper final : public client::context
{
using inherited::get;
};
Added: subversion/trunk/subversion/bindings/cxx/src/private/debug_private.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/private/debug_private.hpp?rev=1850753&view=auto
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/private/debug_private.hpp
(added)
+++ subversion/trunk/subversion/bindings/cxx/src/private/debug_private.hpp Tue
Jan 8 14:29:31 2019
@@ -0,0 +1,49 @@
+/**
+ * @copyright
+ * ====================================================================
+ * 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.
+ * ====================================================================
+ * @endcopyright
+ */
+
+#ifndef SVNXX_PRIVATE_DEBUG_HPP
+#define SVNXX_PRIVATE_DEBUG_HPP
+
+// We can only write pool debug logs with SVN_DEBUG enabled.
+#if defined(SVNXX_POOL_DEBUG) && !defined(SVN_DEBUG)
+# undef SVNXX_POOL_DEBUG
+#endif
+
+#ifdef SVNXX_POOL_DEBUG
+#include "private/svn_debug.h"
+#endif
+
+namespace apache {
+namespace subversion {
+namespace svnxx {
+namespace impl {
+
+extern const char root_pool_tag[];
+extern const char root_pool_key[];
+
+} // namespace impl
+} // namespace svnxx
+} // namespace subversion
+} // namespace apache
+
+#endif // SVNXX_PRIVATE_DEBUG_HPP
Propchange:
subversion/trunk/subversion/bindings/cxx/src/private/debug_private.hpp
------------------------------------------------------------------------------
svn:eol-style = native
Modified: subversion/trunk/subversion/bindings/cxx/src/private/init_private.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/private/init_private.hpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/private/init_private.hpp
(original)
+++ subversion/trunk/subversion/bindings/cxx/src/private/init_private.hpp Tue
Jan 8 14:29:31 2019
@@ -33,33 +33,31 @@
#include "svnxx/init.hpp"
#include "svnxx/detail/noncopyable.hpp"
-#include "svn_private_config.h"
-
namespace apache {
namespace subversion {
namespace svnxx {
namespace detail {
-class context : noncopyable
+class global_state : noncopyable
{
public:
- using ptr = std::shared_ptr<context>;
- using weak_ptr = std::weak_ptr<context>;
+ using ptr = std::shared_ptr<global_state>;
+ using weak_ptr = std::weak_ptr<global_state>;
- ~context();
+ ~global_state();
static ptr create();
static ptr get()
{
- auto ctx = self.lock();
- if (!ctx)
+ auto state = self.lock();
+ if (!state)
{
throw std::logic_error(
- _("The SVN++ library is not initialized."
- " Did you forget to create an instance of "
- " the apache::subversion::svnxx::init class?"));
+ "The SVN++ library is not initialized."
+ " Did you forget to create an instance of "
+ " the apache::subversion::svnxx::init class?");
}
- return ctx;
+ return state;
}
apr_pool_t* get_root_pool() const noexcept
@@ -68,8 +66,8 @@ public:
}
private:
- // Thou shalt not create contexts other than through the factory.
- context();
+ // Thou shalt not create global_states other than through the factory.
+ global_state();
apr_pool_t* root_pool{nullptr};
Modified: subversion/trunk/subversion/bindings/cxx/tests/test_init.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/tests/test_init.cpp?rev=1850753&r1=1850752&r2=1850753&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/tests/test_init.cpp (original)
+++ subversion/trunk/subversion/bindings/cxx/tests/test_init.cpp Tue Jan 8
14:29:31 2019
@@ -27,38 +27,38 @@ namespace detail = ::apache::subversion:
BOOST_AUTO_TEST_SUITE(init);
-BOOST_AUTO_TEST_CASE(context_with_init)
+BOOST_AUTO_TEST_CASE(state_with_init)
{
svn::init svnxx_initialized;
- BOOST_TEST(detail::context::get());
+ BOOST_TEST(detail::global_state::get());
}
-BOOST_AUTO_TEST_CASE(context_without_init)
+BOOST_AUTO_TEST_CASE(state_without_init)
{
- BOOST_CHECK_THROW(detail::context::get(), std::logic_error);
+ BOOST_CHECK_THROW(detail::global_state::get(), std::logic_error);
}
BOOST_AUTO_TEST_CASE(init_scope)
{
{
svn::init svnxx_initialized;
- BOOST_TEST(detail::context::get());
+ BOOST_TEST(detail::global_state::get());
}
- BOOST_CHECK_THROW(detail::context::get(), std::logic_error);
+ BOOST_CHECK_THROW(detail::global_state::get(), std::logic_error);
}
-BOOST_AUTO_TEST_CASE(multi_init_same_context)
+BOOST_AUTO_TEST_CASE(multi_init_same_state)
{
svn::init svnxx_initialized_first;
- const auto ctx = detail::context::get();
- BOOST_REQUIRE(ctx);
+ const auto state = detail::global_state::get();
+ BOOST_REQUIRE(state);
{
svn::init svnxx_initialized_second;
- BOOST_TEST(ctx == detail::context::get());
+ BOOST_TEST(state == detail::global_state::get());
}
- BOOST_TEST(ctx == detail::context::get());
+ BOOST_TEST(state == detail::global_state::get());
}
BOOST_AUTO_TEST_SUITE_END();
