From b7173ac94a4ca0522c4391f067c1823543a098cb Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Sun, 20 Aug 2017 13:04:11 +0200 Subject: [PATCH 1/9] Print out example launch intent when executed without any argument --- src/anbox/cmds/launch.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/anbox/cmds/launch.cpp b/src/anbox/cmds/launch.cpp index ac3b277..10137c2 100644 --- a/src/anbox/cmds/launch.cpp +++ b/src/anbox/cmds/launch.cpp @@ -84,6 +84,8 @@ anbox::cmds::Launch::Launch() action([this](const cli::Command::Context&) { if (!intent_.valid()) { ERROR("The intent you provided is invalid. Please provide a correct launch intent."); + ERROR("For example to launch the application manager, run:"); + ERROR("$ anbox launch --package=org.anbox.appmgr --component=org.anbox.appmgr.AppViewActivity"); return EXIT_FAILURE; } From 6b3295f8a803ccdbdb8f4836e2e8b62f8df4e1ac Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Sun, 20 Aug 2017 14:03:24 +0200 Subject: [PATCH 2/9] Only attempt to create a renderer when we don't have one already --- src/anbox/ui/splash_screen.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/anbox/ui/splash_screen.cpp b/src/anbox/ui/splash_screen.cpp index b52c817..d5b9b48 100644 --- a/src/anbox/ui/splash_screen.cpp +++ b/src/anbox/ui/splash_screen.cpp @@ -18,6 +18,7 @@ #include "anbox/ui/splash_screen.h" #include "anbox/config.h" #include "anbox/utils.h" +#include "anbox/logger.h" #include @@ -44,9 +45,15 @@ SplashScreen::SplashScreen() { SDL_FillRect(surface, nullptr, SDL_MapRGB(surface->format, 0xee, 0xee, 0xee)); SDL_UpdateWindowSurface(window_); - auto renderer = SDL_CreateRenderer(window_, -1, SDL_RENDERER_ACCELERATED); - if (!renderer) - BOOST_THROW_EXCEPTION(std::runtime_error("Could not create renderer")); + auto renderer = SDL_GetRenderer(window_); + if (!renderer) { + DEBUG("Window has no associated renderer yet, creating one ..."); + renderer = SDL_CreateRenderer(window_, -1, SDL_RENDERER_ACCELERATED); + if (!renderer) { + const auto msg = utils::string_format("Could not create renderer: %s", SDL_GetError()); + BOOST_THROW_EXCEPTION(std::runtime_error(msg)); + } + } const auto icon_path = utils::string_format("%s/ui/loading-screen.png", SystemConfiguration::instance().resource_dir()); auto img = IMG_LoadTexture(renderer, icon_path.c_str()); From 07b32235ea04d5023631dc33beaee5d173dca30a Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Sun, 20 Aug 2017 14:04:30 +0200 Subject: [PATCH 3/9] Don't force host GL driver on nvidia platforms anymore Using the host GL driver is already the default so need to special case this for nvidia. --- src/anbox/cmds/session_manager.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/anbox/cmds/session_manager.cpp b/src/anbox/cmds/session_manager.cpp index 9f632ee..bd7f809 100644 --- a/src/anbox/cmds/session_manager.cpp +++ b/src/anbox/cmds/session_manager.cpp @@ -133,13 +133,6 @@ anbox::cmds::SessionManager::SessionManager() return EXIT_FAILURE; } - // If we're running with the properietary nvidia driver we always - // use the host EGL driver as our translation doesn't work here. - if (fs::exists("/dev/nvidiactl")) { - INFO("Detected properietary nvidia driver; forcing use of the host EGL driver."); - gles_driver_ = graphics::GLRendererServer::Config::Driver::Host; - } - utils::ensure_paths({ SystemConfiguration::instance().socket_dir(), SystemConfiguration::instance().input_device_dir(), From 579d28865bc78696b086e75ba1a9600c2d168b4d Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Tue, 22 Aug 2017 08:15:54 +0200 Subject: [PATCH 4/9] cmds: apply proper daemonization handling for forked session manager --- src/anbox/cmds/launch.cpp | 47 ++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/anbox/cmds/launch.cpp b/src/anbox/cmds/launch.cpp index 10137c2..7762af3 100644 --- a/src/anbox/cmds/launch.cpp +++ b/src/anbox/cmds/launch.cpp @@ -38,6 +38,21 @@ namespace { const boost::posix_time::seconds max_wait_timeout{240}; const int max_restart_attempts{3}; const std::chrono::seconds restart_interval{5}; + +static int redirect_to_null(int flags, int fd) { + int fd2; + if ((fd2 = open("/dev/null", flags)) < 0) + return -1; + + if (fd2 == fd) + return fd; + + if (dup2(fd2, fd) < 0) + return -1; + + close(fd2); + return fd; +} } bool anbox::cmds::Launch::try_launch_activity(const std::shared_ptr &stub) { @@ -144,12 +159,34 @@ anbox::cmds::Launch::Launch() } try { - auto flags = core::posix::StandardStream::stdout | core::posix::StandardStream::stderr; - // If we have logging enable in debug mode then we allow the child process - // to print to stdout/stderr too. - if (Log().GetSeverity() == Logger::Severity::kDebug) - flags = core::posix::StandardStream::empty; + auto flags = core::posix::StandardStream::empty; auto child = core::posix::fork([&]() { + // We redirect all in/out/err to /dev/null as they can't be seen + // anywhere. All logging output will directly go to syslog as we + // will become a session leader below which will get us rid of a + // controlling terminal. + if (redirect_to_null(O_RDONLY, 0) < 0 || + redirect_to_null(O_WRONLY, 1) < 0 || + redirect_to_null(O_WRONLY, 2) < 0) { + ERROR("Failed to redirect stdout/stderr/stdin: %s", strerror(errno)); + return core::posix::exit::Status::failure; + } + + // As we forked one time already we're sure that our process is + // not the session leader anymore so we can safely become the + // new one and lead the process group. + if (setsid() < 0) { + ERROR("Failed to become new session leader: %s", strerror(errno)); + return core::posix::exit::Status::failure; + } + + umask(0077); + + if (chdir("/") < 0) { + ERROR("Failed to change current directory: %s", strerror(errno)); + return core::posix::exit::Status::failure; + } + auto grandchild = core::posix::exec(exe_path, args, env, flags); grandchild.dont_kill_on_cleanup(); return core::posix::exit::Status::success; From 05fb78e5bfa8ef352d67edc94d393a9426b316bc Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Tue, 22 Aug 2017 08:16:25 +0200 Subject: [PATCH 5/9] logger: use syslog for output when we don't have a controlling tty --- src/anbox/logger.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/anbox/logger.cpp b/src/anbox/logger.cpp index 2e184d8..93b7e10 100644 --- a/src/anbox/logger.cpp +++ b/src/anbox/logger.cpp @@ -25,12 +25,14 @@ #include #include #include +#define BOOST_LOG_USE_NATIVE_SYSLOG +#include #include "anbox/logger.h" namespace { namespace attrs { -BOOST_LOG_ATTRIBUTE_KEYWORD(Severity, "anbox::Severity", anbox::Logger::Severity) +BOOST_LOG_ATTRIBUTE_KEYWORD(Severity, "Severity", anbox::Logger::Severity) BOOST_LOG_ATTRIBUTE_KEYWORD(Location, "Location", anbox::Logger::Location) BOOST_LOG_ATTRIBUTE_KEYWORD(Timestamp, "Timestamp", boost::posix_time::ptime) } @@ -39,7 +41,8 @@ struct BoostLogLogger : public anbox::Logger { BoostLogLogger() : initialized_(false) {} void Init(const anbox::Logger::Severity& severity = anbox::Logger::Severity::kWarning) override { - if (initialized_) return; + if (initialized_) + return; boost::log::formatter formatter = boost::log::expressions::stream @@ -53,8 +56,21 @@ struct BoostLogLogger : public anbox::Logger { << boost::log::expressions::smessage; boost::log::core::get()->remove_all_sinks(); - auto logger = boost::log::add_console_log(std::cout); - logger->set_formatter(formatter); + + // If we have a controlling tty then we use the console for log outpu + // and otherwise we move everything into the system syslog. + if (isatty(0)) { + auto logger = boost::log::add_console_log(std::cout); + logger->set_formatter(formatter); + } else { + boost::shared_ptr backend( + new boost::log::sinks::syslog_backend( + boost::log::keywords::facility = boost::log::sinks::syslog::user, + boost::log::keywords::use_impl = boost::log::sinks::syslog::native)); + backend->set_severity_mapper(boost::log::sinks::syslog::direct_severity_mapping("Severity")); + boost::log::core::get()->add_sink(boost::make_shared>(backend)); + } severity_ = severity; initialized_ = true; From 4b344ad20475a740d1b984cbdf9a409581b7eaf3 Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Tue, 22 Aug 2017 08:16:53 +0200 Subject: [PATCH 6/9] emugl: print EGL error when eglMakeCurrent fails --- src/anbox/graphics/emugl/Renderer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/anbox/graphics/emugl/Renderer.cpp b/src/anbox/graphics/emugl/Renderer.cpp index 776de49..199775f 100644 --- a/src/anbox/graphics/emugl/Renderer.cpp +++ b/src/anbox/graphics/emugl/Renderer.cpp @@ -655,7 +655,7 @@ bool Renderer::bindContext(HandleType p_context, HandleType p_drawSurface, draw ? draw->getEGLSurface() : EGL_NO_SURFACE, read ? read->getEGLSurface() : EGL_NO_SURFACE, ctx ? ctx->getEGLContext() : EGL_NO_CONTEXT)) { - ERROR("eglMakeCurrent failed"); + ERROR("eglMakeCurrent failed: 0x%04x", s_egl.eglGetError()); return false; } @@ -737,7 +737,7 @@ bool Renderer::bind_locked() { if (!s_egl.eglMakeCurrent(m_eglDisplay, m_pbufSurface, m_pbufSurface, m_pbufContext)) { - ERROR("eglMakeCurrent failed"); + ERROR("eglMakeCurrent failed: 0x%04x", s_egl.eglGetError()); return false; } From e9cd1b7b66c40c3e01eb84a688a3da8a59cf69ae Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Tue, 22 Aug 2017 08:17:10 +0200 Subject: [PATCH 7/9] emugl: correct locking when we draw a new frame --- src/anbox/graphics/emugl/Renderer.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/anbox/graphics/emugl/Renderer.cpp b/src/anbox/graphics/emugl/Renderer.cpp index 199775f..a807131 100644 --- a/src/anbox/graphics/emugl/Renderer.cpp +++ b/src/anbox/graphics/emugl/Renderer.cpp @@ -938,13 +938,14 @@ void Renderer::draw(RendererWindow *window, const Renderable &renderable, bool Renderer::draw(EGLNativeWindowType native_window, const anbox::graphics::Rect &window_frame, const RenderableList &renderables) { + + std::unique_lock l(m_lock); + auto w = m_nativeWindows.find(native_window); if (w == m_nativeWindows.end()) return false; - if (!bindWindow_locked(w->second)) { - m_lock.unlock(); + if (!bindWindow_locked(w->second)) return false; - } setupViewport(w->second, window_frame); s_gles2.glViewport(0, 0, window_frame.width(), window_frame.height()); @@ -959,9 +960,5 @@ bool Renderer::draw(EGLNativeWindowType native_window, unbind_locked(); - m_lock.lock(); - - m_lock.unlock(); - return false; } From f76c039415500f351294d7c6644894474f149993 Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Tue, 22 Aug 2017 08:17:37 +0200 Subject: [PATCH 8/9] dbus: use a larger timeout for launch API calls --- src/anbox/dbus/interface.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/anbox/dbus/interface.h b/src/anbox/dbus/interface.h index dee679c..9335acc 100644 --- a/src/anbox/dbus/interface.h +++ b/src/anbox/dbus/interface.h @@ -39,7 +39,7 @@ struct ApplicationManager { typedef anbox::dbus::interface::ApplicationManager Interface; typedef void ResultType; static inline std::chrono::milliseconds default_timeout() { - return std::chrono::seconds{1}; + return std::chrono::seconds{60}; } }; }; From 8237c963ed0f10d3e00ec277d46a74722424e3b0 Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Thu, 24 Aug 2017 08:26:52 +0200 Subject: [PATCH 9/9] bridge: use five second timeout for all RPC Before we were waiting forever for a reply which never occured in some cases due to errors when processing incoming protobuf messages we now abort the RPC call after a timeout of five seconds to keep operational. --- src/anbox/bridge/android_api_stub.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/anbox/bridge/android_api_stub.cpp b/src/anbox/bridge/android_api_stub.cpp index 4762b15..e08eb30 100644 --- a/src/anbox/bridge/android_api_stub.cpp +++ b/src/anbox/bridge/android_api_stub.cpp @@ -29,6 +29,10 @@ namespace fs = boost::filesystem; +namespace { +constexpr const std::chrono::milliseconds default_rpc_call_timeout{5000}; +} // namespace + namespace anbox { namespace bridge { AndroidApiStub::AndroidApiStub() {} @@ -103,7 +107,9 @@ void AndroidApiStub::launch(const android::Intent &intent, google::protobuf::NewCallback(this, &AndroidApiStub::application_launched, c.get())); - launch_wait_handle_.wait_for_all(); + launch_wait_handle_.wait_for_pending(default_rpc_call_timeout); + if (!launch_wait_handle_.has_result()) + throw std::runtime_error("RPC call timed out"); if (c->response->has_error()) throw std::runtime_error(c->response->error()); } @@ -135,7 +141,9 @@ void AndroidApiStub::set_focused_task(const std::int32_t &id) { google::protobuf::NewCallback( this, &AndroidApiStub::focused_task_set, c.get())); - set_focused_task_handle_.wait_for_all(); + set_focused_task_handle_.wait_for_pending(default_rpc_call_timeout); + if (!set_focused_task_handle_.has_result()) + throw std::runtime_error("RPC call timed out"); if (c->response->has_error()) throw std::runtime_error(c->response->error()); } @@ -162,7 +170,9 @@ void AndroidApiStub::remove_task(const std::int32_t &id) { google::protobuf::NewCallback( this, &AndroidApiStub::task_removed, c.get())); - remove_task_handle_.wait_for_all(); + remove_task_handle_.wait_for_pending(default_rpc_call_timeout); + if (!remove_task_handle_.has_result()) + throw std::runtime_error("RPC call timed out"); if (c->response->has_error()) throw std::runtime_error(c->response->error()); } @@ -198,7 +208,9 @@ void AndroidApiStub::resize_task(const std::int32_t &id, google::protobuf::NewCallback( this, &AndroidApiStub::task_resized, c.get())); - resize_task_handle_.wait_for_all(); + resize_task_handle_.wait_for_pending(default_rpc_call_timeout); + if (!resize_task_handle_.has_result()) + throw std::runtime_error("RPC call timed out"); if (c->response->has_error()) throw std::runtime_error(c->response->error()); }