Merge pull request #1695 from bitshares/pr-1690-shared-ws-connection-ptr

Fix #1690 cli_wallet crashes on quit
bitshares · May 28, 2019 · 14e0f17 · 14e0f17
2 parents 4194083 + 2389c81
commit 14e0f17
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 54 deletions.
diff --git a/libraries/app/application.cpp b/libraries/app/application.cpp
@@ -246,7 +246,7 @@ std::vector<fc::ip::endpoint> application_impl::resolve_string_to_ip_endpoints(c
 
 void application_impl::new_connection( const fc::http::websocket_connection_ptr& c )
 {
-   auto wsc = std::make_shared<fc::rpc::websocket_api_connection>(*c, GRAPHENE_NET_MAX_NESTED_OBJECTS);
+   auto wsc = std::make_shared<fc::rpc::websocket_api_connection>(c, GRAPHENE_NET_MAX_NESTED_OBJECTS);
    auto login = std::make_shared<graphene::app::login_api>( std::ref(*_self) );
    login->enable_api("database_api");
 

diff --git a/libraries/fc b/libraries/fc
diff --git a/libraries/plugins/delayed_node/delayed_node_plugin.cpp b/libraries/plugins/delayed_node/delayed_node_plugin.cpp
@@ -63,7 +63,9 @@ void delayed_node_plugin::plugin_set_program_options(bpo::options_description& c
 
 void delayed_node_plugin::connect()
 {
-   my->client_connection = std::make_shared<fc::rpc::websocket_api_connection>(*my->client.connect(my->remote_endpoint), GRAPHENE_NET_MAX_NESTED_OBJECTS);
+   my->client_connection = std::make_shared<fc::rpc::websocket_api_connection>(
+                              my->client.connect(my->remote_endpoint),
+                              GRAPHENE_NET_MAX_NESTED_OBJECTS );
    my->database_api = my->client_connection->get_remote_api<graphene::app::database_api>(0);
    my->client_connection_closed = my->client_connection->closed.connect([this] {
       connection_failed();

diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp
@@ -83,7 +83,7 @@
 
 // explicit instantiation for later use
 namespace fc {
-	template class api<graphene::wallet::wallet_api, identity_member>;
+	template class api<graphene::wallet::wallet_api, identity_member_with_optionals>;
 }
 
 #define BRAIN_KEY_WORD_COUNT 16

diff --git a/programs/cli_wallet/main.cpp b/programs/cli_wallet/main.cpp
@@ -193,7 +193,7 @@ int main( int argc, char** argv )
       fc::http::websocket_client client;
       idump((wdata.ws_server));
       auto con  = client.connect( wdata.ws_server );
-      auto apic = std::make_shared<fc::rpc::websocket_api_connection>(*con, GRAPHENE_MAX_NESTED_OBJECTS);
+      auto apic = std::make_shared<fc::rpc::websocket_api_connection>(con, GRAPHENE_MAX_NESTED_OBJECTS);
 
       auto remote_api = apic->get_remote_api< login_api >(1);
       edump((wdata.ws_user)(wdata.ws_password) );
@@ -205,32 +205,11 @@ int main( int argc, char** argv )
 
       fc::api<wallet_api> wapi(wapiptr);
 
-      auto wallet_cli = std::make_shared<fc::rpc::cli>( GRAPHENE_MAX_NESTED_OBJECTS );
-      for( auto& name_formatter : wapiptr->get_result_formatters() )
-         wallet_cli->format_result( name_formatter.first, name_formatter.second );
-
-      boost::signals2::scoped_connection closed_connection(con->closed.connect([wallet_cli]{
-         cerr << "Server has disconnected us.\n";
-         wallet_cli->stop();
-      }));
-      (void)(closed_connection);
-
-      if( wapiptr->is_new() )
-      {
-         std::cout << "Please use the set_password method to initialize a new wallet before continuing\n";
-         wallet_cli->set_prompt( "new >>> " );
-      } else
-         wallet_cli->set_prompt( "locked >>> " );
-
-      boost::signals2::scoped_connection locked_connection(wapiptr->lock_changed.connect([&](bool locked) {
-         wallet_cli->set_prompt(  locked ? "locked >>> " : "unlocked >>> " );
-      }));
-
       auto _websocket_server = std::make_shared<fc::http::websocket_server>();
       if( options.count("rpc-endpoint") )
       {
          _websocket_server->on_connection([&wapi]( const fc::http::websocket_connection_ptr& c ){
-            auto wsc = std::make_shared<fc::rpc::websocket_api_connection>(*c, GRAPHENE_MAX_NESTED_OBJECTS);
+            auto wsc = std::make_shared<fc::rpc::websocket_api_connection>(c, GRAPHENE_MAX_NESTED_OBJECTS);
             wsc->register_api(wapi);
             c->set_session_data( wsc );
          });
@@ -247,19 +226,21 @@ int main( int argc, char** argv )
       if( options.count("rpc-tls-endpoint") )
       {
          _websocket_tls_server->on_connection([&wapi]( const fc::http::websocket_connection_ptr& c ){
-            auto wsc = std::make_shared<fc::rpc::websocket_api_connection>(*c, GRAPHENE_MAX_NESTED_OBJECTS);
+            auto wsc = std::make_shared<fc::rpc::websocket_api_connection>(c, GRAPHENE_MAX_NESTED_OBJECTS);
             wsc->register_api(wapi);
             c->set_session_data( wsc );
          });
-         ilog( "Listening for incoming TLS RPC requests on ${p}", ("p", options.at("rpc-tls-endpoint").as<string>() ));
+         ilog( "Listening for incoming TLS RPC requests on ${p}",
+               ("p", options.at("rpc-tls-endpoint").as<string>()) );
          _websocket_tls_server->listen( fc::ip::endpoint::from_string(options.at("rpc-tls-endpoint").as<string>()) );
          _websocket_tls_server->start_accept();
       }
 
       auto _http_server = std::make_shared<fc::http::server>();
       if( options.count("rpc-http-endpoint" ) )
       {
-         ilog( "Listening for incoming HTTP RPC requests on ${p}", ("p", options.at("rpc-http-endpoint").as<string>() ) );
+         ilog( "Listening for incoming HTTP RPC requests on ${p}",
+               ("p", options.at("rpc-http-endpoint").as<string>()) );
          _http_server->listen( fc::ip::endpoint::from_string( options.at( "rpc-http-endpoint" ).as<string>() ) );
          //
          // due to implementation, on_request() must come AFTER listen()
@@ -276,43 +257,92 @@ int main( int argc, char** argv )
 
       if( !options.count( "daemon" ) )
       {
-         wallet_cli->register_api( wapi );
-         wallet_cli->start();
+         auto wallet_cli = std::make_shared<fc::rpc::cli>( GRAPHENE_MAX_NESTED_OBJECTS );
+         for( auto& name_formatter : wapiptr->get_result_formatters() )
+            wallet_cli->format_result( name_formatter.first, name_formatter.second );
 
-         fc::set_signal_handler([](int signal) {
-            ilog( "Captured SIGINT not in daemon mode" );
-            fclose(stdin);
-         }, SIGINT);
-
-         fc::set_signal_handler([](int signal) {
-            ilog( "Captured SIGTERM not in daemon mode" );
-            fclose(stdin);
-         }, SIGTERM);
+         if( wapiptr->is_new() )
+         {
+            std::cout << "Please use the set_password method to initialize a new wallet before continuing\n";
+            wallet_cli->set_prompt( "new >>> " );
+         }
+         else
+            wallet_cli->set_prompt( "locked >>> " );
+
+         boost::signals2::scoped_connection locked_connection( wapiptr->lock_changed.connect(
+            [wallet_cli](bool locked) {
+               wallet_cli->set_prompt( locked ? "locked >>> " : "unlocked >>> " );
+            }));
+
+         auto sig_set = fc::set_signal_handler( [wallet_cli](int signal) {
+            ilog( "Captured SIGINT not in daemon mode, exiting" );
+            fc::set_signal_handler( [](int sig) {}, SIGINT ); // reinstall an empty SIGINT handler
+            wallet_cli->cancel();
+         }, SIGINT );
+
+         fc::set_signal_handler( [wallet_cli,sig_set](int signal) {
+            ilog( "Captured SIGTERM not in daemon mode, exiting" );
+            sig_set->cancel();
+            fc::set_signal_handler( [](int sig) {}, SIGINT ); // reinstall an empty SIGINT handler
+            wallet_cli->cancel();
+         }, SIGTERM );
+
+         fc::set_signal_handler( [wallet_cli,sig_set](int signal) {
+            ilog( "Captured SIGQUIT not in daemon mode, exiting" );
+            sig_set->cancel();
+            fc::set_signal_handler( [](int sig) {}, SIGINT ); // reinstall an empty SIGINT handler
+            wallet_cli->cancel();
+         }, SIGQUIT );
+
+         boost::signals2::scoped_connection closed_connection( con->closed.connect( [wallet_cli,sig_set] {
+            elog( "Server has disconnected us." );
+            sig_set->cancel();
+            fc::set_signal_handler( [](int sig) {}, SIGINT ); // reinstall an empty SIGINT handler
+            wallet_cli->cancel();
+         }));
 
+         wallet_cli->register_api( wapi );
+         wallet_cli->start();
          wallet_cli->wait();
+
+         locked_connection.disconnect();
+         closed_connection.disconnect();
       }
       else
       {
-        fc::promise<int>::ptr exit_promise = new fc::promise<int>("UNIX Signal Handler");
-        fc::set_signal_handler([&exit_promise](int signal) {
-           exit_promise->set_value(signal);
-        }, SIGINT);
+         fc::promise<int>::ptr exit_promise = new fc::promise<int>("UNIX Signal Handler");
+
+         fc::set_signal_handler( [&exit_promise](int signal) {
+            ilog( "Captured SIGINT in daemon mode, exiting" );
+            exit_promise->set_value(signal);
+         }, SIGINT );
+
+         fc::set_signal_handler( [&exit_promise](int signal) {
+            ilog( "Captured SIGTERM in daemon mode, exiting" );
+            exit_promise->set_value(signal);
+         }, SIGTERM );
+
+         fc::set_signal_handler( [&exit_promise](int signal) {
+            ilog( "Captured SIGQUIT in daemon mode, exiting" );
+            exit_promise->set_value(signal);
+         }, SIGQUIT );
+
+         boost::signals2::scoped_connection closed_connection( con->closed.connect( [&exit_promise] {
+            elog( "Server has disconnected us." );
+            exit_promise->set_value(0);
+         }));
 
-        fc::set_signal_handler([&exit_promise](int signal) {
-           exit_promise->set_value(signal);
-        }, SIGTERM);
+         ilog( "Entering Daemon Mode, ^C to exit" );
+         exit_promise->wait();
 
-        ilog( "Entering Daemon Mode, ^C to exit" );
-        exit_promise->wait();
+         closed_connection.disconnect();
       }
 
       wapi->save_wallet_file(wallet_file.generic_string());
-      locked_connection.disconnect();
-      closed_connection.disconnect();
    }
    catch ( const fc::exception& e )
    {
-      std::cout << e.to_detail_string() << "\n";
+      std::cerr << e.to_detail_string() << "\n";
       return -1;
    }
    return 0;

diff --git a/tests/cli/main.cpp b/tests/cli/main.cpp
@@ -217,7 +217,8 @@ class client_connection
       wallet_data.ws_password = "";
       websocket_connection  = websocket_client.connect( wallet_data.ws_server );
 
-      api_connection = std::make_shared<fc::rpc::websocket_api_connection>(*websocket_connection, GRAPHENE_MAX_NESTED_OBJECTS);
+      api_connection = std::make_shared<fc::rpc::websocket_api_connection>( websocket_connection,
+                                                                            GRAPHENE_MAX_NESTED_OBJECTS );
 
       remote_login_api = api_connection->get_remote_api< graphene::app::login_api >(1);
       BOOST_CHECK(remote_login_api->login( wallet_data.ws_user, wallet_data.ws_password ) );
+0 −1		.travis.yml
+3 −7		CMakeLists.txt
+84 −6		include/fc/api.hpp
+5 −2		include/fc/interprocess/signals.hpp
+7 −0		include/fc/network/http/websocket.hpp
+14 −6		include/fc/rpc/api_connection.hpp
+2 −0		include/fc/rpc/cli.hpp
+4 −3		include/fc/rpc/websocket_api.hpp
+1 −1		include/fc/shared_ptr.hpp
+5 −0		include/fc/thread/thread.hpp
+3 −0		src/crypto/openssl.cpp
+12 −11		src/interprocess/signals.cpp
+52 −74		src/network/http/websocket.cpp
+130 −54		src/rpc/cli.cpp
+38 −20		src/rpc/websocket_api.cpp
+23 −3		src/thread/thread.cpp
+12 −4		src/thread/thread_d.hpp
+1 −4		tests/CMakeLists.txt
+0 −191		tests/api.cpp
+138 −0		tests/api_tests.cpp
+9 −44		tests/network/http/websocket_test.cpp
+1 −1		vendor/editline