Make RunApplication() return a MojoResult and TerminateApplication() take one.
R=vardhan@google.com
Review URL: https://codereview.chromium.org/2011593002 .
diff --git a/examples/spinning_cube/spinning_cube_app.cc b/examples/spinning_cube/spinning_cube_app.cc
index adcef63..eac07b2 100644
--- a/examples/spinning_cube/spinning_cube_app.cc
+++ b/examples/spinning_cube/spinning_cube_app.cc
@@ -95,6 +95,5 @@
MojoResult MojoMain(MojoHandle application_request) {
examples::SpinningCubeApp spinning_cube_app;
- mojo::RunApplication(application_request, &spinning_cube_app);
- return MOJO_RESULT_OK;
+ return mojo::RunApplication(application_request, &spinning_cube_app);
}
diff --git a/mojo/public/cpp/application/lib/run_application.cc b/mojo/public/cpp/application/lib/run_application.cc
index 82d3089..03373ef 100644
--- a/mojo/public/cpp/application/lib/run_application.cc
+++ b/mojo/public/cpp/application/lib/run_application.cc
@@ -4,17 +4,66 @@
#include "mojo/public/cpp/application/run_application.h"
+#include <assert.h>
+#include <pthread.h>
+
#include "mojo/public/cpp/application/application_impl_base.h"
+#include "mojo/public/cpp/system/macros.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "mojo/public/cpp/utility/run_loop.h"
#include "mojo/public/interfaces/application/application.mojom.h"
namespace mojo {
-void RunApplication(MojoHandle application_request_handle,
- ApplicationImplBase* application_impl) {
- // TODO(vtl): Possibly we should have an assertion that we're not running, but
- // that requires TLS.
+namespace {
+
+// We store a pointer to a |ResultHolder|, which just stores a |MojoResult|, in
+// TLS so that |TerminateApplication()| can provide the result that
+// |RunApplication()| will return. (The |ResultHolder| is just on
+// |RunApplication()|'s stack.)
+struct ResultHolder {
+#ifndef NDEBUG
+ bool is_set = false;
+#endif
+ MojoResult result = MOJO_RESULT_UNKNOWN;
+};
+
+pthread_key_t g_current_result_holder_key;
+
+// Ensures that we have a TLS slot to store the current result in.
+void InitializeCurrentResultHolderIfNecessary() {
+ static pthread_once_t current_result_holder_key_once = PTHREAD_ONCE_INIT;
+ int error = pthread_once(¤t_result_holder_key_once, []() {
+ int error = pthread_key_create(&g_current_result_holder_key, nullptr);
+ MOJO_ALLOW_UNUSED_LOCAL(error);
+ assert(!error);
+ });
+ MOJO_ALLOW_UNUSED_LOCAL(error);
+ assert(!error);
+}
+
+ResultHolder* GetCurrentResultHolder() {
+ InitializeCurrentResultHolderIfNecessary();
+ return static_cast<ResultHolder*>(
+ pthread_getspecific(g_current_result_holder_key));
+}
+
+void SetCurrentResultHolder(ResultHolder* result_holder) {
+ InitializeCurrentResultHolderIfNecessary();
+
+ int error = pthread_setspecific(g_current_result_holder_key, result_holder);
+ MOJO_ALLOW_UNUSED_LOCAL(error);
+ assert(!error);
+}
+
+} // namespace
+
+MojoResult RunApplication(MojoHandle application_request_handle,
+ ApplicationImplBase* application_impl) {
+ assert(!GetCurrentResultHolder());
+
+ ResultHolder result_holder;
+ SetCurrentResultHolder(&result_holder);
RunLoop loop;
application_impl->Bind(InterfaceRequest<Application>(
@@ -23,10 +72,26 @@
// TODO(vtl): Should we unbind stuff here? (Should there be "will start"/"did
// stop" notifications to the |ApplicationImplBase|?)
+
+ SetCurrentResultHolder(nullptr);
+
+ // TODO(vtl): We'd like to enable the following assertion, but we do things
+ // like |RunLoop::current()->Quit()| in various places.
+ // assert(result_holder.is_set);
+
+ return result_holder.result;
}
-void TerminateApplication() {
+void TerminateApplication(MojoResult result) {
RunLoop::current()->Quit();
+
+ ResultHolder* result_holder = GetCurrentResultHolder();
+ assert(result_holder);
+ assert(!result_holder->is_set);
+ result_holder->result = result;
+#ifndef NDEBUG
+ result_holder->is_set = true;
+#endif
}
} // namespace mojo
diff --git a/mojo/public/cpp/application/run_application.h b/mojo/public/cpp/application/run_application.h
index 98f0ef1..5bfbbd5 100644
--- a/mojo/public/cpp/application/run_application.h
+++ b/mojo/public/cpp/application/run_application.h
@@ -13,15 +13,15 @@
class ApplicationImplBase;
// Sets up a message (run) loop and runs the provided application
-// implementation.
+// implementation. The return value will be the one provided to
+// |TerminateApplication()|.
//
// Typical use (where |MyApplicationImpl| is an implementation of
// |ApplicationImplBase|):
//
// MojoResult MojoMain(MojoHandle application_request_handle) {
// MyApplicationImpl my_app;
-// mojo::RunApplication(application_request_handle, &my_app);
-// return MOJO_RESULT_OK;
+// return mojo::RunApplication(application_request_handle, &my_app);
// }
//
// Note that this may actually be used on any thread (that is not already
@@ -30,23 +30,21 @@
// |*application_impl| must remain alive until the message loop starts running
// (thus it may own itself).
//
-// TODO(vtl): Possibly this should return a |MojoResult| and
-// |TerminateApplication()| should take a |MojoResult| argument (which this
-// returns), but to communicate that value requires TLS.
// TODO(vtl): Maybe this should be like |Environment|, with different possible
// implementations (e.g., "standalone" vs "chromium"). However,
// |ApplicationRunnerChromium| currently has additional goop specific to the
-// main thread. Probably we should have additional "MainRunApplication()" and
-// "MainTerminateApplication()" functions.
-void RunApplication(MojoHandle application_request_handle,
- ApplicationImplBase* application_impl);
+// main thread. Probably we should have additional "RunMainApplication()" and
+// "TerminateMainApplication()" functions.
+MojoResult RunApplication(MojoHandle application_request_handle,
+ ApplicationImplBase* application_impl);
// Terminates the currently-running application. This may only be called from
// "inside" |RunApplication()| (i.e., it is on the stack, which means that the
// message loop is running). This will cause |RunApplication()| to return "soon"
-// (assuming the message loop is not blocked processing some task). This may
-// cause queued work to *not* be executed.
-void TerminateApplication();
+// (assuming the message loop is not blocked processing some task), with return
+// value |result|. This may cause queued work to *not* be executed. This should
+// be executed at most once (per |RunApplication()|).
+void TerminateApplication(MojoResult result);
} // namespace mojo
diff --git a/mojo/public/cpp/bindings/tests/versioning_test_service.cc b/mojo/public/cpp/bindings/tests/versioning_test_service.cc
index 3f7d503..7c59787 100644
--- a/mojo/public/cpp/bindings/tests/versioning_test_service.cc
+++ b/mojo/public/cpp/bindings/tests/versioning_test_service.cc
@@ -121,6 +121,5 @@
MojoResult MojoMain(MojoHandle application_request) {
mojo::test::versioning::HumanResourceSystemServer hr_system_server;
- mojo::RunApplication(application_request, &hr_system_server);
- return MOJO_RESULT_OK;
+ return mojo::RunApplication(application_request, &hr_system_server);
}
diff --git a/mojo/public/cpp/utility/lib/run_loop.cc b/mojo/public/cpp/utility/lib/run_loop.cc
index 1fde95c..2f3178d 100644
--- a/mojo/public/cpp/utility/lib/run_loop.cc
+++ b/mojo/public/cpp/utility/lib/run_loop.cc
@@ -24,7 +24,7 @@
// Ensures that the "current run loop" functionality is available (i.e., that we
// have a TLS slot).
-void EnsureCurrentRunLoopInitialized() {
+void InitializeCurrentRunLoopIfNecessary() {
static pthread_once_t current_run_loop_key_once = PTHREAD_ONCE_INIT;
int error = pthread_once(¤t_run_loop_key_once, []() {
int error = pthread_key_create(&g_current_run_loop_key, nullptr);
@@ -36,7 +36,7 @@
}
void SetCurrentRunLoop(RunLoop* run_loop) {
- EnsureCurrentRunLoopInitialized();
+ InitializeCurrentRunLoopIfNecessary();
int error = pthread_setspecific(g_current_run_loop_key, run_loop);
MOJO_ALLOW_UNUSED_LOCAL(error);
@@ -71,7 +71,7 @@
// static
RunLoop* RunLoop::current() {
- EnsureCurrentRunLoopInitialized();
+ InitializeCurrentRunLoopIfNecessary();
return static_cast<RunLoop*>(pthread_getspecific(g_current_run_loop_key));
}
diff --git a/mojo/services/log/cpp/log_client.h b/mojo/services/log/cpp/log_client.h
index 049ed16..4c10c5c 100644
--- a/mojo/services/log/cpp/log_client.h
+++ b/mojo/services/log/cpp/log_client.h
@@ -25,8 +25,7 @@
//
// MojoResult MojoMain(MojoHandle app_request) {
// MyApp app;
-// mojo::RunApplication(&app);
-// return MOJO_RESULT_OK;
+// return mojo::RunApplication(&app);
// }
#ifndef MOJO_SERVICES_LOG_CPP_LOG_CLIENT_H_
diff --git a/services/test_service/test_request_tracker_application.cc b/services/test_service/test_request_tracker_application.cc
index 2c41f76..9a0f612 100644
--- a/services/test_service/test_request_tracker_application.cc
+++ b/services/test_service/test_request_tracker_application.cc
@@ -43,6 +43,5 @@
MojoResult MojoMain(MojoHandle application_request) {
mojo::test::TestRequestTrackerApplication app;
- mojo::RunApplication(application_request, &app);
- return MOJO_RESULT_OK;
+ return mojo::RunApplication(application_request, &app);
}
diff --git a/services/test_service/test_service_application.cc b/services/test_service/test_service_application.cc
index c883cef..af116a3 100644
--- a/services/test_service/test_service_application.cc
+++ b/services/test_service/test_service_application.cc
@@ -44,7 +44,7 @@
assert(ref_count_ > 0);
ref_count_--;
if (ref_count_ <= 0)
- TerminateApplication();
+ TerminateApplication(MOJO_RESULT_OK);
}
} // namespace test
@@ -52,6 +52,5 @@
MojoResult MojoMain(MojoHandle application_request) {
mojo::test::TestServiceApplication app;
- mojo::RunApplication(application_request, &app);
- return MOJO_RESULT_OK;
+ return mojo::RunApplication(application_request, &app);
}
