stock/Class: allow continuing canceled requests

src/stock/BasicStock.cxx

@@ -14,17 +14,42 @@ struct BasicStock::Create final
 {
 	BasicStock &stock;
 
-	StockGetHandler &handler;
+	/**
+	 * Request was canceled if this field is nullptr.
+	 */
+	StockGetHandler *handler;
 
 	CancellablePointer cancel_ptr;
 
+	const bool continue_on_cancel;
+
 	Create(BasicStock &_stock,
+	       bool _continue_on_cancel,
 	       StockGetHandler &_handler, CancellablePointer &_cancel_ptr) noexcept
-		:stock(_stock), handler(_handler)
+		:stock(_stock), handler(&_handler),
+		 continue_on_cancel(_continue_on_cancel)
 	{
 		_cancel_ptr = *this;
 	}
 
+	bool IsDetached() const noexcept {
+		return handler == nullptr;
+	}
+
+	void Detach() noexcept {
+		assert(handler != nullptr);
+
+		handler = nullptr;
+	}
+
+	void Attach(StockGetHandler &_handler, CancellablePointer &_cancel_ptr) noexcept {
+		assert(handler == nullptr);
+
+		handler = &_handler;
+		_cancel_ptr = *this;
+	}
+
+	// virtual methods from class StockGetHandler
 	[[noreturn]]
 	void OnStockItemReady(StockItem &) noexcept override {
 		// unreachable
@@ -37,7 +62,10 @@ struct BasicStock::Create final
 		std::terminate();
 	}
 
+	// virtual methods from class Cancellable
 	void Cancel() noexcept override {
+		assert(handler != nullptr);
+
 		stock.CreateCanceled(*this);
 	}
 };
@@ -147,12 +175,20 @@ BasicStock::BasicStock(EventLoop &event_loop, StockClass &_cls,
 
 BasicStock::~BasicStock() noexcept
 {
-	assert(create.empty());
-
 	/* must not delete the Stock when there are busy items left */
 	assert(busy.empty());
 
 	ClearIdle();
+
+	create.clear_and_dispose([](Create *c){
+		/* by now, all create operations must have been
+		   canceled */
+		assert(c->handler == nullptr);
+		assert(c->cancel_ptr);
+
+		c->cancel_ptr.Cancel();
+		delete c;
+	});
 }
 
 StockItem *
@@ -215,7 +251,16 @@ BasicStock::GetCreate(StockRequest request,
 		      StockGetHandler &get_handler,
 		      CancellablePointer &cancel_ptr) noexcept
 {
-	auto *c = new Create(*this, get_handler, cancel_ptr);
+	for (auto &c : create) {
+		if (c.IsDetached()) {
+			c.Attach(get_handler, cancel_ptr);
+			return;
+		}
+	}
+
+	auto *c = new Create(*this,
+			     cls.ShouldContinueOnCancel(request.get()),
+			     get_handler, cancel_ptr);
 	create.push_front(*c);
 
 	try {
@@ -230,32 +275,43 @@ BasicStock::ItemCreateSuccess(StockGetHandler &_handler,
 			      StockItem &item) noexcept
 {
 	auto &c = static_cast<Create &>(_handler);
-	auto &get_handler = c.handler;
+	auto *get_handler = c.handler;
 
 	DeleteCreate(c);
 
-	busy.push_front(item);
-
-	get_handler.OnStockItemReady(item);
+	if (get_handler != nullptr) {
+		busy.push_front(item);
+		get_handler->OnStockItemReady(item);
+	} else
+		InjectIdle(item);
 }
 
 void
 BasicStock::ItemCreateError(StockGetHandler &_handler,
 			    std::exception_ptr ep) noexcept
 {
 	auto &c = static_cast<Create &>(_handler);
-	auto &get_handler = c.handler;
+	auto *get_handler = c.handler;
 
 	DeleteCreate(c);
 
 	CheckEmpty();
 
-	get_handler.OnStockItemError(ep);
+	if (get_handler != nullptr)
+		get_handler->OnStockItemError(ep);
+
+	// TODO what to do with the error if the request was canceled?
 }
 
 inline void
 BasicStock::CreateCanceled(Create &c) noexcept
 {
+	if (c.continue_on_cancel) {
+		// TOOD connect to waiting item?
+		c.Detach();
+		return;
+	}
+
 	DeleteCreate(c);
 	OnCreateCanceled();
 	CheckEmpty();

src/stock/Class.hxx

@@ -22,6 +22,20 @@ public:
 			    StockGetHandler &handler,
 			    CancellablePointer &cancel_ptr) = 0;
 
+	/**
+	 * Control whether creating a new item should be continued
+	 * even if the caller cancels the operation.  Once that
+	 * finishes, the new item is put on the "idle" list.  This
+	 * feature can be useful if creating an item is expensive (and
+	 * asynchronous), and canceling it throws away a considerable
+	 * amount of effort which should better be used for the next
+	 * request.
+	 */
+	[[gnu::pure]]
+	virtual bool ShouldContinueOnCancel([[maybe_unused]] const void *request) const noexcept {
+		return false;
+	}
+
 	/**
 	 * @return if non-zero, then two consecutive requests with the
 	 * same value are avoided (for fair scheduling)

test/stock/TestStock.cxx

@@ -123,7 +123,6 @@ TEST(Stock, Basic)
 	StockItem *item, *second, *third;
 
 	Instance instance;
-
 	MyStockClass cls;
 	Stock stock(instance.event_loop, cls, "test", 3, 8,
 		    Event::Duration::zero());
@@ -287,3 +286,139 @@ TEST(Stock, Basic)
 	ASSERT_EQ(num_release, 2);
 	ASSERT_EQ(num_destroy, 5);
 }
+
+TEST(Stock, ContinueOnCancel)
+{
+	Instance instance;
+
+	struct CocStockClass final : StockClass, Cancellable {
+		MyStockItem *item = nullptr;
+		StockGetHandler *handler = nullptr;
+
+		unsigned n_create = 0;
+
+		~CocStockClass() noexcept {
+			assert(item == nullptr);
+			assert(handler == nullptr);
+		}
+
+		void Finish() noexcept {
+			assert(item != nullptr);
+			assert(handler != nullptr);
+
+			std::exchange(item, nullptr)->InvokeCreateSuccess(*std::exchange(handler, nullptr));
+		}
+
+		/* virtual methods from class StockClass */
+		void Create(CreateStockItem c,
+			    [[maybe_unused]] StockRequest request,
+			    StockGetHandler &_handler,
+			    CancellablePointer &cancel_ptr) override {
+			assert(item == nullptr);
+			assert(handler == nullptr);
+
+			++n_create;
+
+			handler = &_handler;
+			item = new MyStockItem(c);
+			cancel_ptr = *this;
+		}
+
+		bool ShouldContinueOnCancel([[maybe_unused]] const void *request) const noexcept override {
+			return true;
+		}
+
+		/* virtual methods from class Cancellable */
+		void Cancel() noexcept {
+			assert(item != nullptr);
+			assert(handler != nullptr);
+
+			handler = nullptr;
+			delete item;
+			item = nullptr;
+		}
+	} cls;
+
+	Stock stock(instance.event_loop, cls, "test", 3, 8,
+		    Event::Duration::zero());
+
+	MyStockGetHandler handler;
+	CancellablePointer cancel_ptr;
+
+	// get one, finish, return
+
+	stock.Get(nullptr, handler, cancel_ptr);
+
+	EXPECT_EQ(cls.n_create, 1);
+	EXPECT_FALSE(handler.got_item);
+
+	cls.Finish();
+
+	EXPECT_EQ(cls.n_create, 1);
+	EXPECT_TRUE(handler.got_item);
+
+	stock.Put(*handler.last_item, PutAction::DESTROY);
+
+	// get one, cancel, finish, get again (immediately)
+
+	cls.item = nullptr;
+	cls.handler = nullptr;
+	handler.got_item = false;
+
+	stock.Get(nullptr, handler, cancel_ptr);
+
+	EXPECT_EQ(cls.n_create, 2);
+	EXPECT_FALSE(handler.got_item);
+
+	cancel_ptr.Cancel();
+
+	cls.Finish();
+
+	stock.Get(nullptr, handler, cancel_ptr);
+
+	EXPECT_EQ(cls.n_create, 2);
+	EXPECT_TRUE(handler.got_item);
+
+	stock.Put(*handler.last_item, PutAction::DESTROY);
+
+	// get one, cancel, get again, finish
+
+	cls.item = nullptr;
+	cls.handler = nullptr;
+	handler.got_item = false;
+
+	stock.Get(nullptr, handler, cancel_ptr);
+
+	EXPECT_EQ(cls.n_create, 3);
+	EXPECT_FALSE(handler.got_item);
+
+	cancel_ptr.Cancel();
+
+	stock.Get(nullptr, handler, cancel_ptr);
+
+	EXPECT_EQ(cls.n_create, 3);
+	EXPECT_FALSE(handler.got_item);
+
+	cls.Finish();
+
+	EXPECT_EQ(cls.n_create, 3);
+	EXPECT_TRUE(handler.got_item);
+
+	stock.Put(*handler.last_item, PutAction::DESTROY);
+
+	// get one, cancel and leave (destructor must cancel it)
+
+	cls.item = nullptr;
+	cls.handler = nullptr;
+	handler.got_item = false;
+
+	stock.Get(nullptr, handler, cancel_ptr);
+
+	EXPECT_EQ(cls.n_create, 4);
+	EXPECT_FALSE(handler.got_item);
+
+	cancel_ptr.Cancel();
+
+	EXPECT_EQ(cls.n_create, 4);
+	EXPECT_FALSE(handler.got_item);
+}