From 1ba56adc28006e48ed3763e8002dfd5db0b2341c Mon Sep 17 00:00:00 2001
From: Francois Rousseau <frousseau@google.com>
Date: Sat, 11 May 2019 20:28:15 +0000
Subject: [PATCH] [feedback] spawn a new DataProvider on every request

by having feedback_agent spawn a new process on every fuchsia.feedback.DataProvider
request it gets, we can better isolate each request handling, in
particular if it has dangling threads due to some attachment collection
hanging and make sure the process exits upon request fulfillment

DX-1497 #comment

TESTED=`fx run-test feedback_agent_tests`

Change-Id: I103b36ef2cfc4df6a7eef5a5783ddc22e93357e8
---
 src/developer/crashpad_agent/crashpad_agent.h |  2 +
 src/developer/feedback_agent/BUILD.gn         | 28 +++++++++-
 .../{feedback_agent.cc => data_provider.cc}   | 17 +++---
 .../{feedback_agent.h => data_provider.h}     | 19 +++----
 .../feedback_agent/data_provider_main.cc      | 56 +++++++++++++++++++
 src/developer/feedback_agent/main.cc          | 48 +++++++++++++---
 .../feedback_agent/meta/feedback_agent.cmx    |  1 +
 src/developer/feedback_agent/tests/BUILD.gn   |  8 +--
 ..._unittest.cc => data_provider_unittest.cc} | 22 ++++----
 .../tests/feedback_agent_integration_test.cc  | 30 +++++++---
 ...nittest.cmx => data_provider_unittest.cmx} |  2 +-
 11 files changed, 180 insertions(+), 53 deletions(-)
 rename src/developer/feedback_agent/{feedback_agent.cc => data_provider.cc} (89%)
 rename src/developer/feedback_agent/{feedback_agent.h => data_provider.h} (74%)
 create mode 100644 src/developer/feedback_agent/data_provider_main.cc
 rename src/developer/feedback_agent/tests/{feedback_agent_unittest.cc => data_provider_unittest.cc} (95%)
 rename src/developer/feedback_agent/tests/meta/{feedback_agent_unittest.cmx => data_provider_unittest.cmx} (84%)

diff --git a/src/developer/crashpad_agent/crashpad_agent.h b/src/developer/crashpad_agent/crashpad_agent.h
index adf206bd18a..9b36f8377e2 100644
--- a/src/developer/crashpad_agent/crashpad_agent.h
+++ b/src/developer/crashpad_agent/crashpad_agent.h
@@ -94,6 +94,8 @@ class CrashpadAgent : public Analyzer {
   const std::unique_ptr<crashpad::CrashReportDatabase> database_;
   const std::unique_ptr<CrashServer> crash_server_;
 
+  // TODO(DX-1499): we should have a connection to fuchsia.feedback.DataProvider
+  // per GetData() call, not a single one overall.
   fuchsia::feedback::DataProviderPtr feedback_data_provider_;
 
   FXL_DISALLOW_COPY_AND_ASSIGN(CrashpadAgent);
diff --git a/src/developer/feedback_agent/BUILD.gn b/src/developer/feedback_agent/BUILD.gn
index f0f83cdd753..fbf8273f7c0 100644
--- a/src/developer/feedback_agent/BUILD.gn
+++ b/src/developer/feedback_agent/BUILD.gn
@@ -7,6 +7,7 @@ import("//build/package/component.gni")
 
 package("feedback_agent") {
   deps = [
+    ":data_provider",
     ":main",
   ]
 
@@ -14,11 +15,14 @@ package("feedback_agent") {
     {
       name = "feedback_agent"
     },
+    {
+      name = "data_provider"
+    },
   ]
 
   meta = [
     {
-      path = rebase_path("meta/feedback_agent.cmx")
+      path = "meta/feedback_agent.cmx"
       dest = "feedback_agent.cmx"
     },
   ]
@@ -31,13 +35,31 @@ executable("main") {
     "main.cc",
   ]
 
+  deps = [
+    "//garnet/public/lib/syslog/cpp",
+    "//sdk/fidl/fuchsia.feedback",
+    "//sdk/lib/fidl/cpp",
+    "//sdk/lib/sys/cpp",
+    "//src/lib/fxl",
+    "//zircon/public/lib/async-loop-cpp",
+    "//zircon/public/lib/zx",
+  ]
+}
+
+executable("data_provider") {
+  sources = [
+    "data_provider_main.cc",
+  ]
+
   deps = [
     ":src",
     "//garnet/public/lib/syslog/cpp",
     "//sdk/fidl/fuchsia.feedback",
     "//sdk/lib/fidl/cpp",
     "//sdk/lib/sys/cpp",
+    "//src/lib/fxl",
     "//zircon/public/lib/async-loop-cpp",
+    "//zircon/public/lib/zx",
   ]
 }
 
@@ -47,8 +69,8 @@ source_set("src") {
     "annotations.h",
     "attachments.cc",
     "attachments.h",
-    "feedback_agent.cc",
-    "feedback_agent.h",
+    "data_provider.cc",
+    "data_provider.h",
     "image_conversion.cc",
     "image_conversion.h",
     "log_listener.cc",
diff --git a/src/developer/feedback_agent/feedback_agent.cc b/src/developer/feedback_agent/data_provider.cc
similarity index 89%
rename from src/developer/feedback_agent/feedback_agent.cc
rename to src/developer/feedback_agent/data_provider.cc
index 06124fff016..0068bd98f05 100644
--- a/src/developer/feedback_agent/feedback_agent.cc
+++ b/src/developer/feedback_agent/data_provider.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/developer/feedback_agent/feedback_agent.h"
+#include "src/developer/feedback_agent/data_provider.h"
 
 #include <fuchsia/feedback/cpp/fidl.h>
 #include <fuchsia/ui/scenic/cpp/fidl.h>
@@ -18,11 +18,12 @@
 namespace fuchsia {
 namespace feedback {
 
-FeedbackAgent::FeedbackAgent(async_dispatcher_t* dispatcher,
-                             std::shared_ptr<::sys::ServiceDirectory> services)
+DataProviderImpl::DataProviderImpl(
+    async_dispatcher_t* dispatcher,
+    std::shared_ptr<::sys::ServiceDirectory> services)
     : executor_(dispatcher), services_(services) {}
 
-void FeedbackAgent::GetData(GetDataCallback callback) {
+void DataProviderImpl::GetData(GetDataCallback callback) {
   // Today attachments are fetched asynchronously, but annotations are not.
   // In the future, we can use fit::join_promises() if annotations need to be
   // fetched asynchronously as well.
@@ -54,8 +55,8 @@ void FeedbackAgent::GetData(GetDataCallback callback) {
   executor_.schedule_task(std::move(promise));
 }
 
-void FeedbackAgent::GetScreenshot(ImageEncoding encoding,
-                                  GetScreenshotCallback callback) {
+void DataProviderImpl::GetScreenshot(ImageEncoding encoding,
+                                     GetScreenshotCallback callback) {
   // We add the provided callback to the vector of pending callbacks we maintain
   // and save a reference to pass to the Scenic callback.
   auto& saved_callback = get_png_screenshot_callbacks_.emplace_back(
@@ -95,7 +96,7 @@ void FeedbackAgent::GetScreenshot(ImageEncoding encoding,
       });
 }
 
-void FeedbackAgent::ConnectToScenic() {
+void DataProviderImpl::ConnectToScenic() {
   scenic_ = services_->Connect<fuchsia::ui::scenic::Scenic>();
   scenic_.set_error_handler([this](zx_status_t status) {
     FX_LOGS(ERROR) << "Lost connection to Scenic service";
@@ -103,7 +104,7 @@ void FeedbackAgent::ConnectToScenic() {
   });
 }
 
-void FeedbackAgent::TerminateAllGetScreenshotCallbacks() {
+void DataProviderImpl::TerminateAllGetScreenshotCallbacks() {
   for (const auto& callback : get_png_screenshot_callbacks_) {
     (*callback)(/*screenshot=*/nullptr);
   }
diff --git a/src/developer/feedback_agent/feedback_agent.h b/src/developer/feedback_agent/data_provider.h
similarity index 74%
rename from src/developer/feedback_agent/feedback_agent.h
rename to src/developer/feedback_agent/data_provider.h
index 74c2886542d..e2e8348f93d 100644
--- a/src/developer/feedback_agent/feedback_agent.h
+++ b/src/developer/feedback_agent/data_provider.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef SRC_DEVELOPER_FEEDBACK_AGENT_FEEDBACK_AGENT_H_
-#define SRC_DEVELOPER_FEEDBACK_AGENT_FEEDBACK_AGENT_H_
+#ifndef SRC_DEVELOPER_FEEDBACK_AGENT_DATA_PROVIDER_H_
+#define SRC_DEVELOPER_FEEDBACK_AGENT_DATA_PROVIDER_H_
 
 #include <fuchsia/feedback/cpp/fidl.h>
 #include <fuchsia/ui/scenic/cpp/fidl.h>
@@ -19,16 +19,13 @@ namespace fuchsia {
 namespace feedback {
 
 // Provides data useful to attach in feedback reports (crash or user feedback).
-class FeedbackAgent : public DataProvider {
+class DataProviderImpl : public DataProvider {
  public:
-  FeedbackAgent(async_dispatcher_t* dispatcher,
-                std::shared_ptr<::sys::ServiceDirectory> services);
+  DataProviderImpl(async_dispatcher_t* dispatcher,
+                   std::shared_ptr<::sys::ServiceDirectory> services);
 
-  // Returns all the feedback data except the screenshot, which is provided
-  // separately.
+  // |fuchsia.feedback.DataProvider|
   void GetData(GetDataCallback callback) override;
-
-  // Returns an image of the current view encoded in the provided |encoding|.
   void GetScreenshot(ImageEncoding encoding,
                      GetScreenshotCallback callback) override;
 
@@ -44,6 +41,8 @@ class FeedbackAgent : public DataProvider {
   async::Executor executor_;
   const std::shared_ptr<::sys::ServiceDirectory> services_;
 
+  // TODO(DX-1499): we should have a connection to Scenic per GetScreenshot()
+  // call, not a single one overall.
   fuchsia::ui::scenic::ScenicPtr scenic_;
   // We keep track of the pending GetScreenshot callbacks so we can terminate
   // all of them when we lose the connection with Scenic.
@@ -54,4 +53,4 @@ class FeedbackAgent : public DataProvider {
 }  // namespace feedback
 }  // namespace fuchsia
 
-#endif  // SRC_DEVELOPER_FEEDBACK_AGENT_FEEDBACK_AGENT_H_
+#endif  // SRC_DEVELOPER_FEEDBACK_AGENT_DATA_PROVIDER_H_
diff --git a/src/developer/feedback_agent/data_provider_main.cc b/src/developer/feedback_agent/data_provider_main.cc
new file mode 100644
index 00000000000..6065bf15477
--- /dev/null
+++ b/src/developer/feedback_agent/data_provider_main.cc
@@ -0,0 +1,56 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <fuchsia/feedback/cpp/fidl.h>
+#include <lib/async-loop/cpp/loop.h>
+#include <lib/fidl/cpp/interface_request.h>
+#include <lib/sys/cpp/component_context.h>
+#include <lib/syslog/cpp/logger.h>
+#include <lib/zx/channel.h>
+#include <stdlib.h>
+#include <zircon/errors.h>
+#include <zircon/process.h>
+#include <zircon/processargs.h>
+#include <zircon/status.h>
+
+#include "src/developer/feedback_agent/data_provider.h"
+#include "src/lib/fxl/strings/string_printf.h"
+
+int main(int argc, const char** argv) {
+  syslog::InitLogger({"feedback"});
+
+  // This process is spawned by the feedback_agent process, which forwards it
+  // the incoming request through PA_USER0.
+  fidl::InterfaceRequest<fuchsia::feedback::DataProvider> request(
+      zx::channel(zx_take_startup_handle(PA_HND(PA_USER0, 0))));
+  if (!request.is_valid()) {
+    FX_LOGS(ERROR) << "Invalid incoming fuchsia.feedback.DataProvider request";
+    return EXIT_FAILURE;
+  }
+
+  async::Loop loop(&kAsyncLoopConfigAttachToThread);
+  auto context = sys::ComponentContext::Create();
+  fuchsia::feedback::DataProviderImpl data_provider(loop.dispatcher(),
+                                                    context->svc());
+  fidl::Binding<fuchsia::feedback::DataProvider> binding(&data_provider);
+  // TODO(DX-1497): in addition to exiting the process when the connection is
+  // closed, we should have an internal timeout since the last call and exit the
+  // process then in case clients don't close the connection themselves.
+  binding.set_error_handler([&loop](zx_status_t status) {
+    loop.Shutdown();
+    // We exit successfully when the client closes the connection.
+    if (status == ZX_ERR_PEER_CLOSED) {
+      exit(0);
+    } else {
+      FX_LOGS(ERROR) << fxl::StringPrintf("Received channel error: %d (%s)",
+                                          status, zx_status_get_string(status));
+      exit(1);
+    }
+  });
+  binding.Bind(std::move(request));
+
+  loop.Run();
+
+  return EXIT_SUCCESS;
+}
diff --git a/src/developer/feedback_agent/main.cc b/src/developer/feedback_agent/main.cc
index febe8fbf11a..f9d2c0444a7 100644
--- a/src/developer/feedback_agent/main.cc
+++ b/src/developer/feedback_agent/main.cc
@@ -4,22 +4,56 @@
 
 #include <fuchsia/feedback/cpp/fidl.h>
 #include <lib/async-loop/cpp/loop.h>
-#include <lib/fidl/cpp/binding_set.h>
+#include <lib/fdio/spawn.h>
+#include <lib/fidl/cpp/interface_request.h>
 #include <lib/sys/cpp/component_context.h>
 #include <lib/syslog/cpp/logger.h>
 #include <stdlib.h>
+#include <zircon/errors.h>
+#include <zircon/processargs.h>
+#include <zircon/status.h>
 
-#include "src/developer/feedback_agent/feedback_agent.h"
+#include "src/lib/fxl/strings/string_printf.h"
+
+fidl::InterfaceRequestHandler<fuchsia::feedback::DataProvider>
+SpawnNewDataProvider() {
+  return [](fidl::InterfaceRequest<fuchsia::feedback::DataProvider> request) {
+    // We spawn a new process to which we forward the channel of the incoming
+    // request so it can handle it.
+    //
+    // Note that today we do not keep track of the spawned process.
+    fdio_spawn_action_t actions = {};
+    actions.action = FDIO_SPAWN_ACTION_ADD_HANDLE;
+    actions.h.id = PA_HND(PA_USER0, 0);
+    actions.h.handle = request.TakeChannel().release();
+
+    const char* args[] = {
+        "/pkg/bin/data_provider",
+        nullptr,
+    };
+    char err_msg[FDIO_SPAWN_ERR_MSG_MAX_LENGTH] = {};
+    const zx_status_t spawn_status =
+        fdio_spawn_etc(ZX_HANDLE_INVALID, FDIO_SPAWN_CLONE_ALL, args[0], args,
+                       nullptr, 1, &actions, nullptr, err_msg);
+    if (spawn_status != ZX_OK) {
+      FX_LOGS(ERROR) << fxl::StringPrintf(
+          "Failed to spawn data provider to handle incoming request: %d "
+          "(%s): %s",
+          spawn_status, zx_status_get_string(spawn_status), err_msg);
+    }
+  };
+}
 
 int main(int argc, const char** argv) {
-  syslog::InitLogger({"feedback_agent"});
+  syslog::InitLogger({"feedback"});
 
   async::Loop loop(&kAsyncLoopConfigAttachToThread);
   auto context = sys::ComponentContext::Create();
-  fuchsia::feedback::FeedbackAgent feedback_agent(loop.dispatcher(),
-                                                  context->svc());
-  fidl::BindingSet<fuchsia::feedback::DataProvider> bindings;
-  context->outgoing()->AddPublicService(bindings.GetHandler(&feedback_agent));
+  // We spawn a new process capable of handling fuchsia.feedback.DataProvider
+  // requests on every incoming request. This has the advantage of tying each
+  // request to a different process that can be cleaned up once it is done or
+  // after a timeout and take care of dangling threads for instance, cf. CF-756.
+  context->outgoing()->AddPublicService(SpawnNewDataProvider());
 
   loop.Run();
 
diff --git a/src/developer/feedback_agent/meta/feedback_agent.cmx b/src/developer/feedback_agent/meta/feedback_agent.cmx
index b9ec6b2b102..bd5870f7a6e 100644
--- a/src/developer/feedback_agent/meta/feedback_agent.cmx
+++ b/src/developer/feedback_agent/meta/feedback_agent.cmx
@@ -12,6 +12,7 @@
         "services": [
             "fuchsia.logger.Log",
             "fuchsia.logger.LogSink",
+            "fuchsia.process.Launcher",
             "fuchsia.ui.scenic.Scenic"
         ]
     }
diff --git a/src/developer/feedback_agent/tests/BUILD.gn b/src/developer/feedback_agent/tests/BUILD.gn
index bd1c0581289..3ac333d8759 100644
--- a/src/developer/feedback_agent/tests/BUILD.gn
+++ b/src/developer/feedback_agent/tests/BUILD.gn
@@ -8,7 +8,7 @@ import("//build/testing/environments.gni")
 test_package("feedback_agent_tests") {
   tests = [
     {
-      name = "feedback_agent_unittest"
+      name = "data_provider_unittest"
     },
     {
       name = "log_listener_unittest"
@@ -29,17 +29,17 @@ test_package("feedback_agent_tests") {
   ]
 
   deps = [
+    ":data_provider_unittest",
     ":feedback_agent_integration_test",
-    ":feedback_agent_unittest",
     ":log_listener_unittest",
   ]
 }
 
-executable("feedback_agent_unittest") {
+executable("data_provider_unittest") {
   testonly = true
 
   sources = [
-    "feedback_agent_unittest.cc",
+    "data_provider_unittest.cc",
     "stub_scenic.cc",
     "stub_scenic.h",
   ]
diff --git a/src/developer/feedback_agent/tests/feedback_agent_unittest.cc b/src/developer/feedback_agent/tests/data_provider_unittest.cc
similarity index 95%
rename from src/developer/feedback_agent/tests/feedback_agent_unittest.cc
rename to src/developer/feedback_agent/tests/data_provider_unittest.cc
index 16266a5146a..86ac2a72d56 100644
--- a/src/developer/feedback_agent/tests/feedback_agent_unittest.cc
+++ b/src/developer/feedback_agent/tests/data_provider_unittest.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/developer/feedback_agent/feedback_agent.h"
+#include "src/developer/feedback_agent/data_provider.h"
 
 #include <fuchsia/feedback/cpp/fidl.h>
 #include <fuchsia/math/cpp/fidl.h>
@@ -153,7 +153,7 @@ MATCHER_P2(MatchesAttachment, expected_key, expected_value,
 //
 // This does not test the environment service. It directly instantiates the
 // class, without connecting through FIDL.
-class FeedbackAgentTest : public gtest::RealLoopFixture {
+class DataProviderImplTest : public gtest::RealLoopFixture {
  public:
   void SetUp() override {
     stub_scenic_.reset(new StubScenic());
@@ -163,7 +163,7 @@ class FeedbackAgentTest : public gtest::RealLoopFixture {
     FXL_CHECK(service_directory_provider_.AddService(
                   stub_logger_->GetHandler(dispatcher())) == ZX_OK);
 
-    agent_.reset(new FeedbackAgent(
+    agent_.reset(new DataProviderImpl(
         dispatcher(), service_directory_provider_.service_directory()));
   }
 
@@ -180,7 +180,7 @@ class FeedbackAgentTest : public gtest::RealLoopFixture {
     stub_logger_->set_messages(messages);
   }
 
-  std::unique_ptr<FeedbackAgent> agent_;
+  std::unique_ptr<DataProviderImpl> agent_;
 
  private:
   ::sys::testing::ServiceDirectoryProvider service_directory_provider_;
@@ -189,7 +189,7 @@ class FeedbackAgentTest : public gtest::RealLoopFixture {
   std::unique_ptr<StubLogger> stub_logger_;
 };
 
-TEST_F(FeedbackAgentTest, GetScreenshot_SucceedOnScenicReturningSuccess) {
+TEST_F(DataProviderImplTest, GetScreenshot_SucceedOnScenicReturningSuccess) {
   const size_t image_dim_in_px = 100;
   std::vector<TakeScreenshotResponse> scenic_responses;
   scenic_responses.emplace_back(CreateCheckerboardScreenshot(image_dim_in_px),
@@ -227,7 +227,7 @@ TEST_F(FeedbackAgentTest, GetScreenshot_SucceedOnScenicReturningSuccess) {
   EXPECT_EQ(actual_pixels, expected_pixels);
 }
 
-TEST_F(FeedbackAgentTest, GetScreenshot_FailOnScenicReturningFailure) {
+TEST_F(DataProviderImplTest, GetScreenshot_FailOnScenicReturningFailure) {
   std::vector<TakeScreenshotResponse> scenic_responses;
   scenic_responses.emplace_back(CreateEmptyScreenshot(), kFailure);
   set_scenic_responses(std::move(scenic_responses));
@@ -248,7 +248,7 @@ TEST_F(FeedbackAgentTest, GetScreenshot_FailOnScenicReturningFailure) {
   EXPECT_EQ(feedback_response.screenshot, nullptr);
 }
 
-TEST_F(FeedbackAgentTest,
+TEST_F(DataProviderImplTest,
        GetScreenshot_FailOnScenicReturningNonBGRA8Screenshot) {
   std::vector<TakeScreenshotResponse> scenic_responses;
   scenic_responses.emplace_back(CreateNonBGRA8Screenshot(), kSuccess);
@@ -270,8 +270,8 @@ TEST_F(FeedbackAgentTest,
   EXPECT_EQ(feedback_response.screenshot, nullptr);
 }
 
-TEST_F(FeedbackAgentTest, GetScreenshot_ParallelRequests) {
-  // We simulate three calls to FeedbackAgent::GetScreenshot(): one for which
+TEST_F(DataProviderImplTest, GetScreenshot_ParallelRequests) {
+  // We simulate three calls to DataProviderImpl::GetScreenshot(): one for which
   // the stub Scenic will return a checkerboard 10x10, one for a 20x20 and one
   // failure.
   const size_t num_calls = 3u;
@@ -300,7 +300,7 @@ TEST_F(FeedbackAgentTest, GetScreenshot_ParallelRequests) {
 
   EXPECT_TRUE(get_scenic_responses().empty());
 
-  // We cannot assume that the order of the FeedbackAgent::GetScreenshot()
+  // We cannot assume that the order of the DataProviderImpl::GetScreenshot()
   // calls match the order of the Scenic::TakeScreenshot() callbacks because of
   // the async message loop. Thus we need to match them as sets.
   //
@@ -328,7 +328,7 @@ TEST_F(FeedbackAgentTest, GetScreenshot_ParallelRequests) {
   }
 }
 
-TEST_F(FeedbackAgentTest, GetData_SmokeTest) {
+TEST_F(DataProviderImplTest, GetData_SmokeTest) {
   // CollectSystemLogs() has its own set of unit tests so we only cover one log
   // message here to check that we are attaching the logs.
   set_logger_messages({
diff --git a/src/developer/feedback_agent/tests/feedback_agent_integration_test.cc b/src/developer/feedback_agent/tests/feedback_agent_integration_test.cc
index 55d38607455..734747738f6 100644
--- a/src/developer/feedback_agent/tests/feedback_agent_integration_test.cc
+++ b/src/developer/feedback_agent/tests/feedback_agent_integration_test.cc
@@ -149,19 +149,31 @@ void CheckNumberOfDataProviderProcesses(
   EXPECT_EQ(num_feedback_agents, 1u);
 }
 
-// This test case isn't super useful in itself. This is just to demonstrate how
-// to use the utility helper that will be needed to check the number of spawned
-// data providers, cf. DX-1497.
-TEST_F(FeedbackAgentIntegrationTest, CheckFeedbackAgentSpawned) {
-  DataProviderSyncPtr data_provider;
-  environment_services_->Connect(data_provider.NewRequest());
+TEST_F(FeedbackAgentIntegrationTest, OneDataProviderPerRequest) {
+  DataProviderSyncPtr data_provider_1;
+  environment_services_->Connect(data_provider_1.NewRequest());
   // As the connection is asynchronous, we make a call with the SyncPtr to make
   // sure the connection is established and the process for the service spawned
   // before checking its existence.
   DataProvider_GetData_Result out_result;
-  ASSERT_EQ(data_provider->GetData(&out_result), ZX_OK);
-
-  CheckNumberOfDataProviderProcesses(0u);
+  ASSERT_EQ(data_provider_1->GetData(&out_result), ZX_OK);
+  CheckNumberOfDataProviderProcesses(1u);
+
+  DataProviderSyncPtr data_provider_2;
+  environment_services_->Connect(data_provider_2.NewRequest());
+  ASSERT_EQ(data_provider_2->GetData(&out_result), ZX_OK);
+  CheckNumberOfDataProviderProcesses(2u);
+
+  DataProviderSyncPtr data_provider_3;
+  environment_services_->Connect(data_provider_3.NewRequest());
+  ASSERT_EQ(data_provider_3->GetData(&out_result), ZX_OK);
+  CheckNumberOfDataProviderProcesses(3u);
+
+  data_provider_1.Unbind();
+  data_provider_2.Unbind();
+  data_provider_3.Unbind();
+  // Ideally we would check after each Unbind() that there is one less
+  // data_provider process, but the process clean up is asynchronous.
 }
 
 }  // namespace
diff --git a/src/developer/feedback_agent/tests/meta/feedback_agent_unittest.cmx b/src/developer/feedback_agent/tests/meta/data_provider_unittest.cmx
similarity index 84%
rename from src/developer/feedback_agent/tests/meta/feedback_agent_unittest.cmx
rename to src/developer/feedback_agent/tests/meta/data_provider_unittest.cmx
index 3a25fa36a72..953149cb9cd 100644
--- a/src/developer/feedback_agent/tests/meta/feedback_agent_unittest.cmx
+++ b/src/developer/feedback_agent/tests/meta/data_provider_unittest.cmx
@@ -1,6 +1,6 @@
 {
     "program": {
-        "binary": "test/feedback_agent_unittest"
+        "binary": "test/data_provider_unittest"
     },
     "sandbox": {
         "dev": [
-- 
GitLab