Fix #242 - witness node crash

Rather than using futures and waiting in the destructor, the APIs now
use enable_shared_from_this and the lambda captures a shared pointer to
the API to prevent it from going out of scope.  As a result the
destructor can not be called while there is a pending async operation
which removes the need to wait in the destructor and thereby removing
the potential for an exception to be thrown causing this crash.
This commit is contained in:
Daniel Larimer 2015-08-18 11:36:50 -04:00
parent aa38feef62
commit 7e42d4b3e8
2 changed files with 17 additions and 29 deletions

View file

@ -50,24 +50,6 @@ namespace graphene { namespace app {
database_api::~database_api()
{
elog("freeing database api ${x}", ("x",int64_t(this)) );
try {
if(_broadcast_changes_complete.valid())
{
_broadcast_changes_complete.cancel();
ilog( "waiting..");
_broadcast_changes_complete.wait();
}
if(_broadcast_removed_complete.valid())
{
_broadcast_removed_complete.cancel();
ilog( "waiting..");
_broadcast_removed_complete.wait();
}
ilog( "done" );
} catch (const fc::exception& e)
{
wlog("${e}", ("e",e.to_detail_string()));
}
}
fc::variants database_api::get_objects(const vector<object_id_type>& ids)const
@ -610,15 +592,18 @@ namespace graphene { namespace app {
{
if( _callbacks.size() )
{
/// we need to ensure the database_api is not deleted for the life of the async operation
auto capture_this = shared_from_this();
for( uint32_t trx_num = 0; trx_num < b.transactions.size(); ++trx_num )
{
const auto& trx = b.transactions[trx_num];
auto id = trx.id();
auto itr = _callbacks.find(id);
auto block_num = b.block_num();
if( itr != _callbacks.end() )
{
fc::async( [=](){ itr->second( fc::variant(transaction_confirmation{ id, block_num, trx_num, trx}) ); } );
auto block_num = b.block_num();
auto& callback = _callbacks.find(id)->second;
fc::async( [capture_this,this,id,block_num,trx_num,trx,callback](){ callback( fc::variant(transaction_confirmation{ id, block_num, trx_num, trx}) ); } );
}
}
}
@ -807,6 +792,9 @@ namespace graphene { namespace app {
void database_api::on_objects_removed( const vector<const object*>& objs )
{
/// we need to ensure the database_api is not deleted for the life of the async operation
auto capture_this = shared_from_this();
if( _account_subscriptions.size() )
{
map<account_id_type, vector<variant> > broadcast_queue;
@ -823,7 +811,7 @@ namespace graphene { namespace app {
if( broadcast_queue.size() )
{
_broadcast_removed_complete = fc::async([=](){
fc::async([capture_this,broadcast_queue,this](){
for( const auto& item : broadcast_queue )
{
auto sub = _account_subscriptions.find(item.first);
@ -848,7 +836,7 @@ namespace graphene { namespace app {
}
if( broadcast_queue.size() )
{
_broadcast_removed_complete = fc::async([=](){
fc::async([capture_this,this,broadcast_queue](){
for( const auto& item : broadcast_queue )
{
auto sub = _market_subscriptions.find(item.first);
@ -904,12 +892,12 @@ namespace graphene { namespace app {
}
}
auto capture_this = shared_from_this();
/// TODO: consider making _broadcast_changes_complete a deque and
/// pushing the future back / popping the prior future if it is complete.
/// if a connection hangs then this could get backed up and result in
/// a failure to exit cleanly.
_broadcast_changes_complete = fc::async([=](){
fc::async([capture_this,this,broadcast_queue,market_broadcast_queue,my_objects](){
for( const auto& item : broadcast_queue )
{
edump( (item) );
@ -980,7 +968,9 @@ namespace graphene { namespace app {
if(_market_subscriptions.count(market))
subscribed_markets_ops[market].push_back(std::make_pair(op.op, op.result));
}
fc::async([=](){
/// we need to ensure the database_api is not deleted for the life of the async operation
auto capture_this = shared_from_this();
fc::async([this,capture_this,subscribed_markets_ops](){
for(auto item : subscribed_markets_ops)
{
auto itr = _market_subscriptions.find(item.first);

View file

@ -54,7 +54,7 @@ namespace graphene { namespace app {
* read-only; all modifications to the database must be performed via transactions. Transactions are broadcast via
* the @ref network_broadcast_api.
*/
class database_api
class database_api : public std::enable_shared_from_this<database_api>
{
public:
database_api(graphene::chain::database& db);
@ -383,8 +383,6 @@ namespace graphene { namespace app {
void on_objects_removed(const vector<const object*>& objs);
void on_applied_block();
fc::future<void> _broadcast_changes_complete;
fc::future<void> _broadcast_removed_complete;
boost::signals2::scoped_connection _change_connection;
boost::signals2::scoped_connection _removed_connection;
boost::signals2::scoped_connection _applied_block_connection;
@ -427,7 +425,7 @@ namespace graphene { namespace app {
/**
* @brief The network_broadcast_api class allows broadcasting of transactions.
*/
class network_broadcast_api
class network_broadcast_api : public std::enable_shared_from_this<network_broadcast_api>
{
public:
network_broadcast_api(application& a);