IF: Fix threading issues in qc_chain & chain_pacemaker

[heifner]

Depends on #1523.

Do not access chain state from net threads. Solution: signals from main thread to update internal HotStuff state. Then access to internal state from net threads is protected by mutex.

Also, get finalizer state request (protected by mutex) in http plugin should access finality state data from cache which is updated from threads that are modifying the finality state. Should be consistent to not see partial updates to state.

[ericpassmore]

Comments in qc_chain.cpp

leap/libraries/hotstuff/qc_chain.cpp

Lines 733 to 746 in 2315323

	// Invoked when we could perhaps make a proposal to the network (or to ourselves, if we are the leader).
	void qc_chain::on_beat(){
	// Non-proposing leaders do not care about on_beat(), because leaders react to a block proposal
	// which comes from processing an incoming new block message from a proposer instead.
	// on_beat() is called by the pacemaker, which decides when it's time to check whether we are
	// proposers that should check whether as proposers we should propose a new hotstuff block to
	// the network (or to ourselves, which is faster and doesn't require the bandwidth of an additional
	// gossip round for a new proposed block).
	// The current criteria for a leader selecting a proposal among all proposals it receives is to go
	// with the first valid one that it receives. So if a proposer is also a leader, it silently goes
	// with its own proposal, which is hopefully valid at the point of generation which is also the
	// point of consumption.
	//

Comments in qc_chain.hpp

leap/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp

Lines 149 to 161 in 2315323

	// This mutex synchronizes all writes to the data members of this qc_chain against
	// get_state() calls (which ultimately come from e.g. the HTTP plugin).
	// This could result in a HTTP query that gets the state of the core while it is
	// in the middle of processing a given request, since this is not serializing
	// against high-level message or request processing borders.
	// If that behavior is not desired, we can instead synchronize this against a
	// consistent past snapshot of the qc_chain's state for e.g. the HTTP plugin,
	// which would be updated at the end of processing every request to the core
	// that does alter the qc_chain (hotstuff protocol state).
	// And if the chain_pacemaker::_hotstuff_global_mutex locking strategy is ever
	// changed, then this probably needs to be reviewed as well.
	//
	mutable std::mutex _state_mutex;

Comments in qc_pacemaker.hpp

leap/libraries/hotstuff/include/eosio/hotstuff/chain_pacemaker.hpp

Lines 52 to 64 in 2315323

	// This serializes all messages (high-level requests) to the qc_chain core.
	// For maximum safety, the qc_chain core will only process one request at a time.
	// These requests can come directly from the net threads, or indirectly from a
	// dedicated finalizer thread (TODO: discuss).
	#warning discuss
	std::mutex _hotstuff_global_mutex;
	chain::controller* _chain = nullptr;
	qc_chain _qc_chain;
	uint32_t _quorum_threshold = 15; //FIXME/TODO: calculate from schedule
	};

[fcecin]

Note (1): the upcoming PR for this moves everything in qc_chain that isn't an interface to private: as discussed in the last meeting, as that helped with solving threading.

Note (2): the "Non-proposing leaders" / on_beat() comment is not a threading issue so I'm not addressing it in the upcoming PR (this item can be moved to another ticket if needed).

Status: after validating the solution to the get_state cache (the overhead ended up just incrementing an uint64 inside the global engine mutex + qc_chain is now 100% lock-free internally + R/W lock at the cache that is at the chain_pacemaker now + cache update check itself is lock-free), I will work on "do not access chain state from net threads" next.

[heifner]

Completed by #1550 & #1574