Fix inventory caching bug causing crash when resuming craft

[jbytheway]

Summary

SUMMARY: Bugfixes "Fix crash related to tool usage on resumed crafts"

Purpose of change

Fixes #32786.
Fixes #32694.

Consider the following chunk of code (which is on the stacktrace from the above issues):

Cataclysm-DDA/src/requirements.cpp

Lines 969 to 997 in 764aa9f

	[&craft_components]( std::vector<item_comp> &comps ) {
	item_comp &comp = comps.front();
	if( item::count_by_charges( comp.type ) && comp.count > 0 ) {
	int qty = craft_components.charges_of( comp.type, comp.count );
	comp.count -= qty;
	// This is terrible but inventory doesn't have a use_charges() function so...
	std::vector<item *> del;
	craft_components.visit_items( [&comp, &qty, &del]( item * e ) {
	std::list<item> used;
	if( e->use_charges( comp.type, qty, used, tripoint_zero ) ) {
	del.push_back( e );
	}
	return qty > 0 ? VisitResponse::SKIP : VisitResponse::ABORT;
	} );
	craft_components.remove_items_with( [&del]( const item & e ) {
	for( const item *it : del ) {
	if( it == &e ) {
	return true;
	}
	}
	return false;
	} );
	} else {
	int amount = craft_components.amount_of( comp.type, comp.count );
	comp.count -= amount;
	craft_components.use_amount( comp.type, amount );
	}
	return comp.count <= 0;
	} ), ret.components.end() );

Line 972 is using visitable<inventory>::charges_of, which includes the following:

Cataclysm-DDA/src/visitable.cpp

Lines 816 to 827 in 764aa9f

	template <>
	int visitable<inventory>::charges_of( const std::string &what, int limit,
	const std::function<bool( const item & )> &filter,
	std::function<void( int )> visitor ) const
	{
	if( what == "UPS" ) {
	int qty = 0;
	qty = sum_no_wrap( qty, charges_of( "UPS_off" ) );
	qty = sum_no_wrap( qty, static_cast<int>( charges_of( "adv_UPS_off" ) / 0.6 ) );
	return std::min( qty, limit );
	}
	const auto &binned = static_cast<const inventory *>( this )->get_binned_items();

That relies on a cache of items binned by type in inventory. That binned cache keeps item* to the inventory contents hanging around.

Back in the first snippet, line 983 calls visitable::remove_items_with, which (potentially) deletes some items from the inventory. However, it doesn't invalidate the binning cache, so dangling pointers can remain.

These lines are within a loop, so the next time around the loop visitable<inventory>::charges_of uses the cache again, and dereferences an invalid pointer in a call to item::is_tool here:

Cataclysm-DDA/src/visitable.cpp

Line 774 in 764aa9f

if( e->is_tool() ) {

Describe the solution

Invalidate the inventory binning cache in visitable<inventory>::remove_items_with.

Add a regression test.

Testing

Ran new regression test under ASan before and after fix. It finds the use-after-free without the fix, and it's gone with the fix.