[everything] Fix circular shared pointer references.

We had a cycle from transaction_manager <-> space_map, and also from
the ref_counters back up to the tm.

This prevented objects being destroyed when various programs exited.

From now on we'll try and only use a shared ptr if ownership is
implied.  Otherwise a reference will be used (eg, for up pointers).
This commit is contained in:
Joe Thornber
2014-08-26 11:14:49 +01:00
parent 930cc9d412
commit a7c96c0e1e
31 changed files with 391 additions and 406 deletions

View File

@@ -172,7 +172,7 @@ namespace persistent_data {
unsigned visit_array_block(ValueVisitor &vv,
btree_path const &p,
typename block_traits::value_type const &v) const {
rblock rb(tm_->read_lock(v, validator_), rc_);
rblock rb(tm_.read_lock(v, validator_), rc_);
for (uint32_t i = 0; i < rb.nr_entries(); i++)
vv.visit(p[0] * rb.max_entries() + i, rb.get(i));
@@ -207,8 +207,6 @@ namespace persistent_data {
unsigned entries_per_block_;
};
typedef typename persistent_data::transaction_manager::ptr tm_ptr;
typedef block_manager<>::write_ref write_ref;
typedef block_manager<>::read_ref read_ref;
@@ -219,23 +217,23 @@ namespace persistent_data {
typedef typename ValueTraits::value_type value_type;
typedef typename ValueTraits::ref_counter ref_counter;
array(tm_ptr tm, ref_counter rc)
array(transaction_manager &tm, ref_counter rc)
: tm_(tm),
entries_per_block_(rblock::calc_max_entries()),
nr_entries_(0),
block_rc_(tm->get_sm(), *this),
block_rc_(tm.get_sm(), *this),
block_tree_(tm, block_rc_),
rc_(rc),
validator_(new array_detail::array_block_validator) {
}
array(tm_ptr tm, ref_counter rc,
array(transaction_manager &tm, ref_counter rc,
block_address root,
unsigned nr_entries)
: tm_(tm),
entries_per_block_(rblock::calc_max_entries()),
nr_entries_(nr_entries),
block_rc_(tm->get_sm(), *this),
block_rc_(tm.get_sm(), *this),
block_tree_(tm, root, block_rc_),
rc_(rc),
validator_(new array_detail::array_block_validator) {
@@ -393,7 +391,7 @@ namespace persistent_data {
wblock new_ablock(unsigned ablock_index) {
uint64_t key[1] = {ablock_index};
write_ref b = tm_->new_block(validator_);
write_ref b = tm_.new_block(validator_);
block_address location = b.get_location();
wblock wb(b, rc_);
@@ -404,13 +402,13 @@ namespace persistent_data {
rblock get_ablock(unsigned ablock_index) const {
block_address addr = lookup_block_address(ablock_index);
return rblock(tm_->read_lock(addr, validator_), rc_);
return rblock(tm_.read_lock(addr, validator_), rc_);
}
wblock shadow_ablock(unsigned ablock_index) {
uint64_t key[1] = {ablock_index};
block_address addr = lookup_block_address(ablock_index);
std::pair<write_ref, bool> p = tm_->shadow(addr, validator_);
std::pair<write_ref, bool> p = tm_.shadow(addr, validator_);
wblock wb = wblock(p.first, rc_);
if (p.second)
@@ -422,11 +420,11 @@ namespace persistent_data {
}
void dec_ablock_entries(block_address addr) {
rblock b(tm_->read_lock(addr, validator_), rc_);
rblock b(tm_.read_lock(addr, validator_), rc_);
b.dec_all_entries();
}
tm_ptr tm_;
transaction_manager &tm_;
unsigned entries_per_block_;
unsigned nr_entries_;
block_ref_counter block_rc_;

View File

@@ -33,12 +33,12 @@ namespace persistent_data {
typedef boost::shared_ptr<bitset_impl> ptr;
typedef persistent_data::transaction_manager::ptr tm_ptr;
bitset_impl(tm_ptr tm)
bitset_impl(transaction_manager &tm)
: nr_bits_(0),
array_(tm, rc_) {
}
bitset_impl(tm_ptr tm, block_address root, unsigned nr_bits)
bitset_impl(transaction_manager &tm, block_address root, unsigned nr_bits)
: nr_bits_(nr_bits),
array_(tm, rc_, root, nr_bits / BITS_PER_ULL) {
}
@@ -203,12 +203,12 @@ namespace persistent_data {
//----------------------------------------------------------------
persistent_data::bitset::bitset(tm_ptr tm)
persistent_data::bitset::bitset(transaction_manager &tm)
: impl_(new bitset_impl(tm))
{
}
persistent_data::bitset::bitset(tm_ptr tm, block_address root, unsigned nr_bits)
persistent_data::bitset::bitset(transaction_manager &tm, block_address root, unsigned nr_bits)
: impl_(new bitset_impl(tm, root, nr_bits))
{
}

View File

@@ -49,10 +49,10 @@ namespace persistent_data {
class bitset {
public:
typedef boost::shared_ptr<bitset> ptr;
typedef persistent_data::transaction_manager::ptr tm_ptr;
bitset(tm_ptr tm);
bitset(tm_ptr tm, block_address root, unsigned nr_bits);
bitset(transaction_manager &tm);
bitset(transaction_manager &tm,
block_address root, unsigned nr_bits);
block_address get_root() const;
unsigned get_nr_bits() const;
void grow(unsigned new_nr_bits, bool default_value);

View File

@@ -34,7 +34,7 @@ namespace {
//----------------------------------------------------------------
bloom_filter::bloom_filter(tm_ptr tm,
bloom_filter::bloom_filter(transaction_manager &tm,
unsigned nr_bits, unsigned nr_probes)
: tm_(tm),
bits_(tm),
@@ -45,7 +45,7 @@ bloom_filter::bloom_filter(tm_ptr tm,
bits_.grow(nr_bits, false);
}
bloom_filter::bloom_filter(tm_ptr tm, block_address root,
bloom_filter::bloom_filter(transaction_manager &tm, block_address root,
unsigned nr_bits, unsigned nr_probes)
: tm_(tm),
bits_(tm, root, nr_bits),

View File

@@ -12,13 +12,12 @@ namespace persistent_data {
class bloom_filter {
public:
typedef boost::shared_ptr<bloom_filter> ptr;
typedef persistent_data::transaction_manager::ptr tm_ptr;
// nr_bits must be a power of two
bloom_filter(tm_ptr tm,
bloom_filter(transaction_manager &tm,
unsigned nr_bits, unsigned nr_probes);
bloom_filter(tm_ptr tm, block_address root,
bloom_filter(transaction_manager &tm, block_address root,
unsigned nr_bits_power, unsigned nr_probes);
block_address get_root() const;
@@ -34,7 +33,7 @@ namespace persistent_data {
void fill_probes(block_address b, vector<unsigned> &probes) const;
tm_ptr tm_;
transaction_manager &tm_;
persistent_data::bitset bits_;
unsigned nr_probes_;
uint64_t mask_;

View File

@@ -198,7 +198,7 @@ namespace persistent_data {
class ro_spine : private boost::noncopyable {
public:
ro_spine(transaction_manager::ptr tm,
ro_spine(transaction_manager &tm,
bcache::validator::ptr v)
: tm_(tm),
validator_(v) {
@@ -212,7 +212,7 @@ namespace persistent_data {
}
private:
transaction_manager::ptr tm_;
transaction_manager &tm_;
bcache::validator::ptr validator_;
std::list<block_manager<>::read_ref> spine_;
};
@@ -223,7 +223,7 @@ namespace persistent_data {
typedef transaction_manager::write_ref write_ref;
typedef boost::optional<block_address> maybe_block;
shadow_spine(transaction_manager::ptr tm,
shadow_spine(transaction_manager &tm,
bcache::validator::ptr v)
: tm_(tm),
@@ -276,7 +276,7 @@ namespace persistent_data {
}
private:
transaction_manager::ptr tm_;
transaction_manager &tm_;
bcache::validator::ptr validator_;
std::list<block_manager<>::write_ref> spine_;
maybe_block root_;
@@ -335,10 +335,10 @@ namespace persistent_data {
typedef typename btree_detail::node_ref<ValueTraits> leaf_node;
typedef typename btree_detail::node_ref<block_traits> internal_node;
btree(typename persistent_data::transaction_manager::ptr tm,
btree(transaction_manager &tm,
typename ValueTraits::ref_counter rc);
btree(typename transaction_manager::ptr tm,
btree(transaction_manager &tm,
block_address root,
typename ValueTraits::ref_counter rc);
@@ -434,7 +434,7 @@ namespace persistent_data {
void inc_children(btree_detail::shadow_spine &spine,
RefCounter &leaf_rc);
typename persistent_data::transaction_manager::ptr tm_;
transaction_manager &tm_;
bool destroy_;
block_address root_;
block_ref_counter internal_rc_;

View File

@@ -64,7 +64,7 @@ namespace persistent_data {
inline void
ro_spine::step(block_address b)
{
spine_.push_back(tm_->read_lock(b, validator_));
spine_.push_back(tm_.read_lock(b, validator_));
if (spine_.size() > 2)
spine_.pop_front();
}
@@ -72,11 +72,11 @@ namespace persistent_data {
inline bool
shadow_spine::step(block_address b)
{
pair<write_ref, bool> p = tm_->shadow(b, validator_);
pair<write_ref, bool> p = tm_.shadow(b, validator_);
try {
step(p.first);
} catch (...) {
tm_->get_sm()->dec(p.first.get_location());
tm_.get_sm()->dec(p.first.get_location());
throw;
}
return p.second;
@@ -392,17 +392,17 @@ namespace persistent_data {
template <unsigned Levels, typename ValueTraits>
btree<Levels, ValueTraits>::
btree(typename transaction_manager::ptr tm,
btree(transaction_manager &tm,
typename ValueTraits::ref_counter rc)
: tm_(tm),
destroy_(false),
internal_rc_(tm->get_sm()),
internal_rc_(tm.get_sm()),
rc_(rc),
validator_(new btree_node_validator)
{
using namespace btree_detail;
write_ref root = tm_->new_block(validator_);
write_ref root = tm_.new_block(validator_);
if (Levels > 1) {
internal_node n = to_node<block_traits>(root);
@@ -424,13 +424,13 @@ namespace persistent_data {
template <unsigned Levels, typename ValueTraits>
btree<Levels, ValueTraits>::
btree(typename transaction_manager::ptr tm,
btree(transaction_manager &tm,
block_address root,
typename ValueTraits::ref_counter rc)
: tm_(tm),
destroy_(false),
root_(root),
internal_rc_(tm->get_sm()),
internal_rc_(tm.get_sm()),
rc_(rc),
validator_(new btree_node_validator)
{
@@ -559,7 +559,7 @@ namespace persistent_data {
typename btree<Levels, ValueTraits>::ptr
btree<Levels, ValueTraits>::clone() const
{
tm_->get_sm()->inc(root_);
tm_.get_sm()->inc(root_);
return ptr(new btree<Levels, ValueTraits>(tm_, root_, rc_));
}
@@ -635,13 +635,13 @@ namespace persistent_data {
node_type type;
unsigned nr_left, nr_right;
write_ref left = tm_->new_block(validator_);
write_ref left = tm_.new_block(validator_);
node_ref<ValueTraits> l = to_node<ValueTraits>(left);
l.set_nr_entries(0);
l.set_max_entries();
l.set_value_size(sizeof(typename ValueTraits::disk_type));
write_ref right = tm_->new_block(validator_);
write_ref right = tm_.new_block(validator_);
node_ref<ValueTraits> r = to_node<ValueTraits>(right);
r.set_nr_entries(0);
r.set_max_entries();
@@ -695,7 +695,7 @@ namespace persistent_data {
node_ref<ValueTraits> l = spine.template get_node<ValueTraits>();
block_address left = spine.get_block();
write_ref right = tm_->new_block(validator_);
write_ref right = tm_.new_block(validator_);
node_ref<ValueTraits> r = to_node<ValueTraits>(right);
unsigned nr_left = l.get_nr_entries() / 2;
@@ -822,14 +822,14 @@ namespace persistent_data {
{
using namespace btree_detail;
read_ref blk = tm_->read_lock(b, validator_);
read_ref blk = tm_.read_lock(b, validator_);
internal_node o = to_node<block_traits>(blk);
// FIXME: use a switch statement
if (o.get_type() == INTERNAL) {
if (v.visit_internal(loc, o)) {
for (unsigned i = 0; i < o.get_nr_entries(); i++)
tm_->prefetch(o.value_at(i));
tm_.prefetch(o.value_at(i));
for (unsigned i = 0; i < o.get_nr_entries(); i++) {
node_location loc2(loc);

View File

@@ -98,7 +98,7 @@ namespace {
typedef transaction_manager::read_ref read_ref;
typedef transaction_manager::write_ref write_ref;
bitmap(transaction_manager::ptr tm,
bitmap(transaction_manager &tm,
index_entry const &ie,
bcache::validator::ptr v)
: tm_(tm),
@@ -107,7 +107,7 @@ namespace {
}
ref_t lookup(unsigned b) const {
read_ref rr = tm_->read_lock(ie_.blocknr_, validator_);
read_ref rr = tm_.read_lock(ie_.blocknr_, validator_);
void const *bits = bitmap_data(rr);
ref_t b1 = test_bit_le(bits, b * 2);
ref_t b2 = test_bit_le(bits, b * 2 + 1);
@@ -117,7 +117,7 @@ namespace {
}
void insert(unsigned b, ref_t n) {
write_ref wr = tm_->shadow(ie_.blocknr_, validator_).first;
write_ref wr = tm_.shadow(ie_.blocknr_, validator_).first;
void *bits = bitmap_data(wr);
bool was_free = !test_bit_le(bits, b * 2) && !test_bit_le(bits, b * 2 + 1);
if (n == 1 || n == 3)
@@ -158,7 +158,7 @@ namespace {
}
void iterate(block_address offset, block_address hi, space_map::iterator &it) const {
read_ref rr = tm_->read_lock(ie_.blocknr_, validator_);
read_ref rr = tm_.read_lock(ie_.blocknr_, validator_);
void const *bits = bitmap_data(rr);
for (unsigned b = 0; b < hi; b++) {
@@ -181,7 +181,7 @@ namespace {
return h + 1;
}
transaction_manager::ptr tm_;
transaction_manager &tm_;
bcache::validator::ptr validator_;
index_entry ie_;
@@ -241,7 +241,7 @@ namespace {
typedef transaction_manager::write_ref write_ref;
sm_disk(index_store::ptr indexes,
transaction_manager::ptr tm)
transaction_manager &tm)
: tm_(tm),
bitmap_validator_(new bitmap_block_validator),
indexes_(indexes),
@@ -251,7 +251,7 @@ namespace {
}
sm_disk(index_store::ptr indexes,
transaction_manager::ptr tm,
transaction_manager &tm,
sm_root const &root)
: tm_(tm),
bitmap_validator_(new bitmap_block_validator),
@@ -354,7 +354,7 @@ namespace {
indexes_->resize(bitmap_count);
for (block_address i = old_bitmap_count; i < bitmap_count; i++) {
write_ref wr = tm_->new_block(bitmap_validator_);
write_ref wr = tm_.new_block(bitmap_validator_);
index_entry ie;
ie.blocknr_ = wr.get_location();
@@ -437,7 +437,7 @@ namespace {
}
protected:
transaction_manager::ptr get_tm() const {
transaction_manager &get_tm() const {
return tm_;
}
@@ -501,7 +501,7 @@ namespace {
ref_counts_.remove(key);
}
transaction_manager::ptr tm_;
transaction_manager &tm_;
bcache::validator::ptr bitmap_validator_;
index_store::ptr indexes_;
block_address nr_blocks_;
@@ -544,12 +544,12 @@ namespace {
public:
typedef boost::shared_ptr<btree_index_store> ptr;
btree_index_store(transaction_manager::ptr tm)
btree_index_store(transaction_manager &tm)
: tm_(tm),
bitmaps_(tm, index_entry_traits::ref_counter()) {
}
btree_index_store(transaction_manager::ptr tm,
btree_index_store(transaction_manager &tm,
block_address root)
: tm_(tm),
bitmaps_(tm, root, index_entry_traits::ref_counter()) {
@@ -592,7 +592,7 @@ namespace {
}
private:
transaction_manager::ptr tm_;
transaction_manager &tm_;
btree<1, index_entry_traits> bitmaps_;
};
@@ -600,13 +600,13 @@ namespace {
public:
typedef boost::shared_ptr<metadata_index_store> ptr;
metadata_index_store(transaction_manager::ptr tm)
metadata_index_store(transaction_manager &tm)
: tm_(tm) {
block_manager<>::write_ref wr = tm_->new_block(index_validator());
block_manager<>::write_ref wr = tm_.new_block(index_validator());
bitmap_root_ = wr.get_location();
}
metadata_index_store(transaction_manager::ptr tm, block_address root, block_address nr_indexes)
metadata_index_store(transaction_manager &tm, block_address root, block_address nr_indexes)
: tm_(tm),
bitmap_root_(root) {
resize(nr_indexes);
@@ -627,7 +627,7 @@ namespace {
virtual void commit_ies() {
std::pair<block_manager<>::write_ref, bool> p =
tm_->shadow(bitmap_root_, index_validator());
tm_.shadow(bitmap_root_, index_validator());
bitmap_root_ = p.first.get_location();
metadata_index *mdi = reinterpret_cast<metadata_index *>(p.first.data());
@@ -661,14 +661,14 @@ namespace {
private:
void load_ies() {
block_manager<>::read_ref rr =
tm_->read_lock(bitmap_root_, index_validator());
tm_.read_lock(bitmap_root_, index_validator());
metadata_index const *mdi = reinterpret_cast<metadata_index const *>(rr.data());
for (unsigned i = 0; i < entries_.size(); i++)
index_entry_traits::unpack(*(mdi->index + i), entries_[i]);
}
transaction_manager::ptr tm_;
transaction_manager &tm_;
block_address bitmap_root_;
std::vector<index_entry> entries_;
};
@@ -677,7 +677,7 @@ namespace {
//----------------------------------------------------------------
checked_space_map::ptr
persistent_data::create_disk_sm(transaction_manager::ptr tm,
persistent_data::create_disk_sm(transaction_manager &tm,
block_address nr_blocks)
{
index_store::ptr store(new btree_index_store(tm));
@@ -688,7 +688,7 @@ persistent_data::create_disk_sm(transaction_manager::ptr tm,
}
checked_space_map::ptr
persistent_data::open_disk_sm(transaction_manager::ptr tm, void *root)
persistent_data::open_disk_sm(transaction_manager &tm, void *root)
{
sm_root_disk d;
sm_root v;
@@ -700,7 +700,7 @@ persistent_data::open_disk_sm(transaction_manager::ptr tm, void *root)
}
checked_space_map::ptr
persistent_data::create_metadata_sm(transaction_manager::ptr tm, block_address nr_blocks)
persistent_data::create_metadata_sm(transaction_manager &tm, block_address nr_blocks)
{
index_store::ptr store(new metadata_index_store(tm));
checked_space_map::ptr sm(new sm_disk(store, tm));
@@ -711,7 +711,7 @@ persistent_data::create_metadata_sm(transaction_manager::ptr tm, block_address n
}
checked_space_map::ptr
persistent_data::open_metadata_sm(transaction_manager::ptr tm, void *root)
persistent_data::open_metadata_sm(transaction_manager &tm, void *root)
{
sm_root_disk d;
sm_root v;

View File

@@ -26,16 +26,16 @@
namespace persistent_data {
checked_space_map::ptr
create_disk_sm(transaction_manager::ptr tm, block_address nr_blocks);
create_disk_sm(transaction_manager &tm, block_address nr_blocks);
checked_space_map::ptr
open_disk_sm(transaction_manager::ptr tm, void *root);
open_disk_sm(transaction_manager &tm, void *root);
checked_space_map::ptr
create_metadata_sm(transaction_manager::ptr tm, block_address nr_blocks);
create_metadata_sm(transaction_manager &tm, block_address nr_blocks);
checked_space_map::ptr
open_metadata_sm(transaction_manager::ptr tm, void *root);
open_metadata_sm(transaction_manager &tm, void *root);
}
//----------------------------------------------------------------