Skip to content

Commit

Permalink
stock/AbstractStock: eliminate ItemCreateAborted()
Browse files Browse the repository at this point in the history
Allocate a Create object which updates the BasicStock state
automatically without requiring an extra method.  This reduces code
complexity and prepares implementing continue-after-cancel.
  • Loading branch information
MaxKellermann committed Oct 16, 2023
1 parent 57c4e44 commit 52e50b7
Show file tree
Hide file tree
Showing 12 changed files with 85 additions and 79 deletions.
2 changes: 1 addition & 1 deletion src/pg/Stock.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class Stock::Item final : public StockItem, Cancellable, AsyncConnectionHandler
void Cancel() noexcept override {
assert(!initialized || defer_initialized.IsPending());

InvokeCreateAborted();
delete this;
}

/* virtual methods from class AsyncConnectionHandler */
Expand Down
7 changes: 0 additions & 7 deletions src/stock/AbstractStock.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,3 @@ AbstractStock::ItemCreateError(StockItem &item, StockGetHandler &get_handler,
ItemCreateError(get_handler, ep);
delete &item;
}

void
AbstractStock::ItemCreateAborted(StockItem &item) noexcept
{
ItemCreateAborted();
delete &item;
}
2 changes: 0 additions & 2 deletions src/stock/AbstractStock.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,5 @@ public:
std::exception_ptr ep) noexcept;
virtual void ItemCreateError(StockGetHandler &get_handler,
std::exception_ptr ep) noexcept = 0;
void ItemCreateAborted(StockItem &item) noexcept;
virtual void ItemCreateAborted() noexcept = 0;
virtual void ItemUncleanFlagCleared() noexcept {}
};
78 changes: 62 additions & 16 deletions src/stock/BasicStock.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,39 @@

#include <cassert>

struct BasicStock::Create final
: IntrusiveListHook<>, StockGetHandler, Cancellable
{
BasicStock &stock;

StockGetHandler &handler;

CancellablePointer cancel_ptr;

Create(BasicStock &_stock,
StockGetHandler &_handler, CancellablePointer &_cancel_ptr) noexcept
:stock(_stock), handler(_handler)
{
_cancel_ptr = *this;
}

[[noreturn]]
void OnStockItemReady(StockItem &) noexcept override {
// unreachable
std::terminate();
}

[[noreturn]]
void OnStockItemError(std::exception_ptr) noexcept override {
// unreachable
std::terminate();
}

void Cancel() noexcept override {
stock.CreateCanceled(*this);
}
};

void
BasicStock::FadeAll() noexcept
{
Expand Down Expand Up @@ -114,7 +147,7 @@ BasicStock::BasicStock(EventLoop &event_loop, StockClass &_cls,

BasicStock::~BasicStock() noexcept
{
assert(num_create == 0);
assert(create.empty());

/* must not delete the Stock when there are busy items left */
assert(busy.empty());
Expand Down Expand Up @@ -182,46 +215,49 @@ BasicStock::GetCreate(StockRequest request,
StockGetHandler &get_handler,
CancellablePointer &cancel_ptr) noexcept
{
++num_create;
auto *c = new Create(*this, get_handler, cancel_ptr);
create.push_front(*c);

try {
cls.Create({*this}, std::move(request),
get_handler, cancel_ptr);
cls.Create({*this}, std::move(request), *c, c->cancel_ptr);
} catch (...) {
ItemCreateError(get_handler, std::current_exception());
ItemCreateError(*c, std::current_exception());
}
}

void
BasicStock::ItemCreateSuccess(StockGetHandler &get_handler,
BasicStock::ItemCreateSuccess(StockGetHandler &_handler,
StockItem &item) noexcept
{
assert(num_create > 0);
--num_create;
auto &c = static_cast<Create &>(_handler);
auto &get_handler = c.handler;

DeleteCreate(c);

busy.push_front(item);

get_handler.OnStockItemReady(item);
}

void
BasicStock::ItemCreateError(StockGetHandler &get_handler,
BasicStock::ItemCreateError(StockGetHandler &_handler,
std::exception_ptr ep) noexcept
{
assert(num_create > 0);
--num_create;
auto &c = static_cast<Create &>(_handler);
auto &get_handler = c.handler;

DeleteCreate(c);

CheckEmpty();

get_handler.OnStockItemError(ep);
}

void
BasicStock::ItemCreateAborted() noexcept
inline void
BasicStock::CreateCanceled(Create &c) noexcept
{
assert(num_create > 0);
--num_create;

DeleteCreate(c);
OnCreateCanceled();
CheckEmpty();
}

Expand Down Expand Up @@ -288,3 +324,13 @@ BasicStock::ItemBusyDisconnect(StockItem &item) noexcept
/* this item will be destroyed by Put() */
item.Fade();
}


inline void
BasicStock::DeleteCreate(Create &c) noexcept
{
assert(!create.empty());

create.erase_and_dispose(create.iterator_to(c),
DeleteDisposer{});
}
30 changes: 19 additions & 11 deletions src/stock/BasicStock.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,17 @@ private:
*/
ItemList busy;

struct Create;

/**
* The number of items that are currently being created. We
* keep track of this because we need to know whether this
* stock is empty (see OnEmpty()) and whether this stock is
* full (see Stock::IsFull()).
* The items that are currently being created. We keep track
* of this because we need to know whether this stock is empty
* (see OnEmpty()) and whether this stock is full (see
* Stock::IsFull()).
*/
std::size_t num_create = 0;
IntrusiveList<Create,
IntrusiveListBaseHookTraits<Create>,
IntrusiveListOptions{.constant_time_size = true}> create;

protected:
bool may_clear = false;
Expand Down Expand Up @@ -104,7 +108,7 @@ public:
*/
[[gnu::pure]]
bool IsEmpty() const noexcept {
return idle.empty() && busy.empty() && num_create == 0;
return idle.empty() && busy.empty() && create.empty();
}

/**
Expand Down Expand Up @@ -133,7 +137,7 @@ public:
ClearIdleIf(predicate);

CheckEmpty();
// TODO: restart the "num_create" list?
// TODO: restart the "create" list?
}

/**
Expand All @@ -149,13 +153,15 @@ protected:

/**
* Determine the number of "active" items, i.e. the busy items
* and the ones being created (#num_create). This number is
* used to compare with the configured #limit.
* and the ones being created (#create). This number is used
* to compare with the configured #limit.
*/
std::size_t GetActiveCount() const noexcept {
return busy.size() + num_create;
return busy.size() + create.size();
}

virtual void OnCreateCanceled() noexcept {}

/**
* The stock has become empty. It is not safe to delete it
* from within this method.
Expand Down Expand Up @@ -225,9 +231,11 @@ public:
StockItem &item) noexcept override;
void ItemCreateError(StockGetHandler &get_handler,
std::exception_ptr ep) noexcept override;
void ItemCreateAborted() noexcept override;

private:
void DeleteCreate(Create &c) noexcept;
void CreateCanceled(Create &c) noexcept;

void ScheduleCleanup() noexcept {
cleanup_event.Schedule(std::chrono::seconds(20));
}
Expand Down
12 changes: 0 additions & 12 deletions src/stock/Item.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ CreateStockItem::InvokeCreateError(StockGetHandler &handler,
stock.ItemCreateError(handler, ep);
}

void
CreateStockItem::InvokeCreateAborted() noexcept
{
stock.ItemCreateAborted();
}

StockItem::~StockItem() noexcept
{
}
Expand Down Expand Up @@ -53,12 +47,6 @@ StockItem::InvokeCreateError(StockGetHandler &handler,
stock.ItemCreateError(*this, handler, ep);
}

void
StockItem::InvokeCreateAborted() noexcept
{
stock.ItemCreateAborted(*this);
}

void
StockItem::InvokeIdleDisconnect() noexcept
{
Expand Down
12 changes: 0 additions & 12 deletions src/stock/Item.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ struct CreateStockItem {
*/
void InvokeCreateError(StockGetHandler &handler,
std::exception_ptr ep) noexcept;

/**
* Announce that the creation of this item has been aborted by the
* caller.
*/
void InvokeCreateAborted() noexcept;
};

class StockItem
Expand Down Expand Up @@ -124,12 +118,6 @@ public:
void InvokeCreateError(StockGetHandler &handler,
std::exception_ptr ep) noexcept;

/**
* Announce that the creation of this item has been aborted by the
* caller.
*/
void InvokeCreateAborted() noexcept;

/**
* Announce that the item has been disconnected by the peer while
* it was idle.
Expand Down
12 changes: 0 additions & 12 deletions src/stock/MultiStock.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,6 @@ MultiStock::OuterItem::ItemCreateError(StockGetHandler &get_handler,
get_handler.OnStockItemError(std::move(ep));
}

void
MultiStock::OuterItem::ItemCreateAborted() noexcept
{
// unreachable
}

void
MultiStock::OuterItem::ItemUncleanFlagCleared() noexcept
{
Expand Down Expand Up @@ -545,12 +539,6 @@ MultiStock::MapItem::ItemCreateError(StockGetHandler &get_handler,
get_handler.OnStockItemError(ep);
}

void
MultiStock::MapItem::ItemCreateAborted() noexcept
{
assert(!get_cancel_ptr);
}

inline void
MultiStock::MapItem::OnLeaseReleased(OuterItem &item) noexcept
{
Expand Down
2 changes: 0 additions & 2 deletions src/stock/MultiStock.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ class MultiStock {
StockItem &item) noexcept override;
void ItemCreateError(StockGetHandler &get_handler,
std::exception_ptr ep) noexcept override;
void ItemCreateAborted() noexcept override;
void ItemUncleanFlagCleared() noexcept override;
};

Expand Down Expand Up @@ -293,7 +292,6 @@ class MultiStock {
StockItem &item) noexcept override;
void ItemCreateError(StockGetHandler &get_handler,
std::exception_ptr ep) noexcept override;
void ItemCreateAborted() noexcept override;

public:
struct Hash {
Expand Down
3 changes: 1 addition & 2 deletions src/stock/Stock.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,8 @@ Stock::ItemCreateError(StockGetHandler &get_handler,
}

void
Stock::ItemCreateAborted() noexcept
Stock::OnCreateCanceled() noexcept
{
BasicStock::ItemCreateAborted();
ScheduleRetryWaiting();
}

Expand Down
3 changes: 2 additions & 1 deletion src/stock/Stock.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,14 @@ public:

void ItemCreateError(StockGetHandler &get_handler,
std::exception_ptr ep) noexcept override;
void ItemCreateAborted() noexcept override;

void ItemUncleanFlagCleared() noexcept override {
ScheduleRetryWaiting();
}

private:
void OnCreateCanceled() noexcept override;

[[gnu::pure]]
WaitingList::iterator PickWaiting() noexcept;

Expand Down
1 change: 0 additions & 1 deletion test/stock/TestMultiStock.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ MyStockClass::DeferredRequest::OnDeferred() noexcept
void
MyStockClass::DeferredRequest::Cancel() noexcept
{
c.InvokeCreateAborted();
delete this;
}

Expand Down

0 comments on commit 52e50b7

Please sign in to comment.