Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(417)

Side by Side Diff: mojo/shell/in_process_dynamic_service_runner.cc

Issue 397733003: mojo: kill app_thread before unloading main app library and add tests (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: also service_manager_unittests Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « mojo/shell/in_process_dynamic_service_runner.h ('k') | mojo/shell/mojo_url_resolver.h » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "mojo/shell/in_process_dynamic_service_runner.h" 5 #include "mojo/shell/in_process_dynamic_service_runner.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/callback_helpers.h" 8 #include "base/callback_helpers.h"
9 #include "base/location.h" 9 #include "base/location.h"
10 #include "base/logging.h" 10 #include "base/logging.h"
11 #include "base/message_loop/message_loop_proxy.h" 11 #include "base/message_loop/message_loop_proxy.h"
12 #include "base/scoped_native_library.h"
13 #include "mojo/public/platform/native/system_thunks.h" 12 #include "mojo/public/platform/native/system_thunks.h"
14 13
15 namespace mojo { 14 namespace mojo {
16 namespace shell { 15 namespace shell {
17 16
18 InProcessDynamicServiceRunner::InProcessDynamicServiceRunner( 17 InProcessDynamicServiceRunner::InProcessDynamicServiceRunner(
19 Context* context) 18 Context* context)
20 : keep_alive_(context), 19 : keep_alive_(context),
21 thread_(this, "app_thread") { 20 thread_(this, "app_thread") {
22 } 21 }
23 22
24 InProcessDynamicServiceRunner::~InProcessDynamicServiceRunner() { 23 InProcessDynamicServiceRunner::~InProcessDynamicServiceRunner() {
25 if (thread_.HasBeenStarted()) { 24 if (thread_.HasBeenStarted()) {
26 DCHECK(!thread_.HasBeenJoined()); 25 DCHECK(!thread_.HasBeenJoined());
27 thread_.Join(); 26 thread_.Join();
28 } 27 }
28
29 // It is important to let the thread exit before unloading the DSO because
30 // the library may have registered thread-local data and destructors to run
31 // on thread termination.
32 app_library_.Reset(base::NativeLibrary());
29 } 33 }
30 34
31 void InProcessDynamicServiceRunner::Start( 35 void InProcessDynamicServiceRunner::Start(
32 const base::FilePath& app_path, 36 const base::FilePath& app_path,
33 ScopedMessagePipeHandle service_handle, 37 ScopedMessagePipeHandle service_handle,
34 const base::Closure& app_completed_callback) { 38 const base::Closure& app_completed_callback) {
35 app_path_ = app_path; 39 app_path_ = app_path;
36 40
37 DCHECK(!service_handle_.is_valid()); 41 DCHECK(!service_handle_.is_valid());
38 service_handle_ = service_handle.Pass(); 42 service_handle_ = service_handle.Pass();
39 43
40 DCHECK(app_completed_callback_runner_.is_null()); 44 DCHECK(app_completed_callback_runner_.is_null());
41 app_completed_callback_runner_ = base::Bind(&base::TaskRunner::PostTask, 45 app_completed_callback_runner_ = base::Bind(&base::TaskRunner::PostTask,
42 base::MessageLoopProxy::current(), 46 base::MessageLoopProxy::current(),
43 FROM_HERE, 47 FROM_HERE,
44 app_completed_callback); 48 app_completed_callback);
45 49
46 DCHECK(!thread_.HasBeenStarted()); 50 DCHECK(!thread_.HasBeenStarted());
47 thread_.Start(); 51 thread_.Start();
48 } 52 }
49 53
50 void InProcessDynamicServiceRunner::Run() { 54 void InProcessDynamicServiceRunner::Run() {
51 DVLOG(2) << "Loading/running Mojo app in process from library: " 55 DVLOG(2) << "Loading/running Mojo app in process from library: "
52 << app_path_.value(); 56 << app_path_.value();
53 57
54 do { 58 do {
55 base::NativeLibraryLoadError error; 59 base::NativeLibraryLoadError error;
56 base::ScopedNativeLibrary app_library( 60 app_library_.Reset(base::LoadNativeLibrary(app_path_, &error));
57 base::LoadNativeLibrary(app_path_, &error)); 61 if (!app_library_.is_valid()) {
58 if (!app_library.is_valid()) {
59 LOG(ERROR) << "Failed to load app library (error: " << error.ToString() 62 LOG(ERROR) << "Failed to load app library (error: " << error.ToString()
60 << ")"; 63 << ")";
61 break; 64 break;
62 } 65 }
63 66
64 MojoSetSystemThunksFn mojo_set_system_thunks_fn = 67 MojoSetSystemThunksFn mojo_set_system_thunks_fn =
65 reinterpret_cast<MojoSetSystemThunksFn>(app_library.GetFunctionPointer( 68 reinterpret_cast<MojoSetSystemThunksFn>(app_library_.GetFunctionPointer(
66 "MojoSetSystemThunks")); 69 "MojoSetSystemThunks"));
67 if (mojo_set_system_thunks_fn) { 70 if (mojo_set_system_thunks_fn) {
68 MojoSystemThunks system_thunks = MojoMakeSystemThunks(); 71 MojoSystemThunks system_thunks = MojoMakeSystemThunks();
69 size_t expected_size = mojo_set_system_thunks_fn(&system_thunks); 72 size_t expected_size = mojo_set_system_thunks_fn(&system_thunks);
70 if (expected_size > sizeof(MojoSystemThunks)) { 73 if (expected_size > sizeof(MojoSystemThunks)) {
71 LOG(ERROR) 74 LOG(ERROR)
72 << "Invalid app library: expected MojoSystemThunks size: " 75 << "Invalid app library: expected MojoSystemThunks size: "
73 << expected_size; 76 << expected_size;
74 break; 77 break;
75 } 78 }
76 } else { 79 } else {
77 // In the component build, Mojo Apps link against mojo_system_impl. 80 // In the component build, Mojo Apps link against mojo_system_impl.
78 #if !defined(COMPONENT_BUILD) 81 #if !defined(COMPONENT_BUILD)
79 // Strictly speaking this is not required, but it's very unusual to have 82 // Strictly speaking this is not required, but it's very unusual to have
80 // an app that doesn't require the basic system library. 83 // an app that doesn't require the basic system library.
81 LOG(WARNING) << "MojoSetSystemThunks not found in app library"; 84 LOG(WARNING) << "MojoSetSystemThunks not found in app library";
82 #endif 85 #endif
83 } 86 }
84 87
85 typedef MojoResult (*MojoMainFunction)(MojoHandle); 88 typedef MojoResult (*MojoMainFunction)(MojoHandle);
86 MojoMainFunction main_function = reinterpret_cast<MojoMainFunction>( 89 MojoMainFunction main_function = reinterpret_cast<MojoMainFunction>(
87 app_library.GetFunctionPointer("MojoMain")); 90 app_library_.GetFunctionPointer("MojoMain"));
88 if (!main_function) { 91 if (!main_function) {
89 LOG(ERROR) << "Entrypoint MojoMain not found"; 92 LOG(ERROR) << "Entrypoint MojoMain not found";
90 break; 93 break;
91 } 94 }
92 95
93 // |MojoMain()| takes ownership of the service handle. 96 // |MojoMain()| takes ownership of the service handle.
94 MojoResult result = main_function(service_handle_.release().value()); 97 MojoResult result = main_function(service_handle_.release().value());
95 if (result < MOJO_RESULT_OK) 98 if (result < MOJO_RESULT_OK)
96 LOG(ERROR) << "MojoMain returned an error: " << result; 99 LOG(ERROR) << "MojoMain returned an error: " << result;
97 } while (false); 100 } while (false);
98 101
99 bool success = app_completed_callback_runner_.Run(); 102 bool success = app_completed_callback_runner_.Run();
100 app_completed_callback_runner_.Reset(); 103 app_completed_callback_runner_.Reset();
101 LOG_IF(ERROR, !success) << "Failed post run app_completed_callback"; 104 LOG_IF(ERROR, !success) << "Failed post run app_completed_callback";
102 } 105 }
103 106
104 } // namespace shell 107 } // namespace shell
105 } // namespace mojo 108 } // namespace mojo
OLDNEW
« no previous file with comments | « mojo/shell/in_process_dynamic_service_runner.h ('k') | mojo/shell/mojo_url_resolver.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698