[Merge] lp:~mandel/location-service/handle-exceptions into lp:location-service
Jim Hodapp
jim.hodapp at canonical.com
Tue Apr 14 17:58:38 UTC 2015
Review: Needs Fixing code
Looks good, just a few typos to fix.
Diff comments:
> === modified file 'src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.cpp'
> --- src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.cpp 2015-01-06 12:35:02 +0000
> +++ src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.cpp 2015-04-14 14:29:11 +0000
> @@ -705,7 +705,24 @@
>
> worker_thread = std::move(std::thread
> {
> - [this]() { system_bus->run(); }
> + [this]()
> + {
> + while(true) {
> + try
> + {
> + system_bus->run();
> + break; // run exited normally
> + }
> + catch (const std::exception& e)
> + {
> + LOG(WARNING) << e.what();
> + }
> + catch(...)
> + {
> + LOG(WARNING) << "Unexpected exception was raised by the io executor.";
> + }
> + }
> + }
> });
> }
>
>
> === modified file 'src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.h'
> --- src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.h 2014-11-10 20:12:56 +0000
> +++ src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.h 2015-04-14 14:29:11 +0000
> @@ -126,7 +126,25 @@
> // And a dedicated worker thread.
> std::thread worker
> {
> - [this]() { service.run(); }
> + [this]()
> + {
> + while(true)
> + {
> + try
> + {
> + service.run();
> + break; // normal exit
> + }
> + catch (const std::exception& e)
> + {
> + LOG(WARNING) << e.what();
> + }
> + catch(...)
> + {
> + LOG(WARNING) << "Received unexpected exception.";
> + }
> + }
> + }
> };
> } dispatcher;
>
>
> === modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.cpp'
> --- src/location_service/com/ubuntu/location/providers/remote/provider.cpp 2015-01-26 19:13:52 +0000
> +++ src/location_service/com/ubuntu/location/providers/remote/provider.cpp 2015-04-14 14:29:11 +0000
> @@ -66,6 +66,7 @@
> try
> {
> service.run();
> + break; // provider was excit correctly
Typo, "was excit" should be "has exited"
> }
> catch (const std::exception& e)
> {
>
> === modified file 'src/location_service/com/ubuntu/location/service/daemon.cpp'
> --- src/location_service/com/ubuntu/location/service/daemon.cpp 2015-02-13 13:19:18 +0000
> +++ src/location_service/com/ubuntu/location/service/daemon.cpp 2015-04-14 14:29:11 +0000
> @@ -15,6 +15,8 @@
> *
> * Authored by: Thomas Voß <thomas.voss at canonical.com>
> */
> +
> +#include <com/ubuntu/location/logging.h>
> #include <com/ubuntu/location/boost_ptree_settings.h>
> #include <com/ubuntu/location/provider_factory.h>
>
> @@ -239,11 +241,52 @@
> };
>
> auto location_service = std::make_shared<location::service::Implementation>(configuration);
> -
> - std::thread t1{[&config](){config.incoming->run();}};
> - std::thread t2{[&config](){config.incoming->run();}};
> - std::thread t3{[&config](){config.incoming->run();}};
> - std::thread t4{[&config](){config.outgoing->run();}};
> + // we need to ensure that is any exception is raised by the executor that we do not let it crash the app
Rephrase to: "We need to ensure that any exception raised by the executor does not crash the app and also gets logged."
> + // and we log the issue
> + auto incomingExecutorRunner = [&config] {
> + while(true)
> + {
> + try
> + {
> + VLOG(10) << "Starting the incoming executor";
> + config.incoming->run();
> + break; // run() exited normally
> + }
> + catch (const std::exception& e)
> + {
> + LOG(WARNING) << e.what();
> + }
> + catch (...)
> + {
> + LOG(WARNING) << "Unexpected exceptions was raised by the incomming dbus executor";
Typo, should be "Unexpected exception was raised by the incoming executor"
> + }
> + }
> + };
> +
> + auto outgoingExecutorRunner = [&config] {
> + while(true)
> + {
> + try
> + {
> + VLOG(10) << "Starting the outgoing executor";
> + config.outgoing->run();
> + break; // run() exited normally
> + }
> + catch (const std::exception& e)
> + {
> + LOG(WARNING) << e.what();
> + }
> + catch (...)
> + {
> + LOG(WARNING) << "Unexpected exceptions was raised by the outgoing dbus executor";
> + }
> + }
> + };
> +
> + std::thread t1{incomingExecutorRunner};
> + std::thread t2{incomingExecutorRunner};
> + std::thread t3{incomingExecutorRunner};
> + std::thread t4{outgoingExecutorRunner};
>
> trap->run();
>
>
--
https://code.launchpad.net/~mandel/location-service/handle-exceptions/+merge/256157
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service.
More information about the Ubuntu-reviews
mailing list