Web MIDI: add dynamic MidiManager instantiation support for Linux

media/midi/midi_manager_alsa.cc

Index: media/midi/midi_manager_alsa.cc
diff --git a/media/midi/midi_manager_alsa.cc b/media/midi/midi_manager_alsa.cc
index d99751bff01be9161f404b4681d980b7b54d839d..101b12b09f8d1a859da83bf961672ced393baa1e 100644
--- a/media/midi/midi_manager_alsa.cc
+++ b/media/midi/midi_manager_alsa.cc
@@ -15,6 +15,7 @@
 
 #include "base/bind.h"
 #include "base/json/json_string_value_serializer.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ -27,6 +28,7 @@
 #include "base/time/time.h"
 #include "crypto/sha2.h"
 #include "media/midi/midi_port_info.h"
+#include "media/midi/midi_service.h"
 
 namespace midi {
 
@@ -35,6 +37,10 @@ namespace {
 using mojom::PortState;
 using mojom::Result;
 
+// TODO(toyoshim): use constexpr for following const values.
+const int kEventTaskRunner = 0;
+const int kSendTaskRunner = 1;
+
 // Per-output buffer. This can be smaller, but then large sysex messages
 // will be (harmlessly) split across multiple seq events. This should
 // not have any real practical effect, except perhaps to slightly reorder
@@ -90,6 +96,17 @@ const unsigned int kCreateInputPortCaps =
 const unsigned int kCreatePortType =
     SND_SEQ_PORT_TYPE_MIDI_GENERIC | SND_SEQ_PORT_TYPE_APPLICATION;
 
+// Global variables to identify MidiManagerAlsa instance.
+const int kInvalidInstanceId = -1;
+int g_active_instance_id = kInvalidInstanceId;
+int g_next_instance_id = 0;
+base::LazyInstance<base::Lock> g_instance_id_lock = LAZY_INSTANCE_INITIALIZER;
+
+// Prevent current instance from quiting Finalize() while tasks run on external
+// TaskRunners.
+base::LazyInstance<base::Lock> g_event_task_lock = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<base::Lock> g_send_task_lock = LAZY_INSTANCE_INITIALIZER;
+
 int AddrToInt(int client, int port) {
   return (client << 8) | port;
 }
@@ -153,8 +170,12 @@ void SetStringIfNonEmpty(base::DictionaryValue* value,
 
 }  // namespace
 
-MidiManagerAlsa::MidiManagerAlsa()
-    : event_thread_("MidiEventThread"), send_thread_("MidiSendThread") {}
+MidiManagerAlsa::MidiManagerAlsa(MidiService* service) : MidiManager(service) {
+  base::AutoLock lock(g_instance_id_lock.Get());
+  CHECK_EQ(kInvalidInstanceId, g_active_instance_id);
+  instance_id_ = g_next_instance_id++;
+  g_active_instance_id = instance_id_;
+}
 
 MidiManagerAlsa::~MidiManagerAlsa() {
   // Take lock to ensure that the members initialized on the IO thread
@@ -169,8 +190,8 @@ MidiManagerAlsa::~MidiManagerAlsa() {
   CHECK(!udev_);
   CHECK(!udev_monitor_);
 
-  CHECK(!send_thread_.IsRunning());
-  CHECK(!event_thread_.IsRunning());
+  base::AutoLock instance_id_lock(g_instance_id_lock.Get());
+  CHECK_EQ(kInvalidInstanceId, g_active_instance_id);
 }
 
 void MidiManagerAlsa::StartInitialization() {
@@ -288,11 +309,10 @@ void MidiManagerAlsa::StartInitialization() {
 
   // Start processing events. Don't do this before enumeration of both
   // ALSA and udev.
-  event_thread_.Start();
-  event_thread_.task_runner()->PostTask(
-      FROM_HERE,
-      base::Bind(&MidiManagerAlsa::ScheduleEventLoop, base::Unretained(this)));
-  send_thread_.Start();
+  service()
+      ->GetTaskRunner(kEventTaskRunner)
+      ->PostTask(FROM_HERE, base::Bind(&MidiManagerAlsa::EventLoop,
+                                       base::Unretained(this), instance_id_));
 
   CompleteInitialization(Result::OK);
 }
@@ -301,23 +321,24 @@ void MidiManagerAlsa::Finalize() {
   base::AutoLock lock(lazy_init_member_lock_);
   DCHECK(initialization_thread_checker_->CalledOnValidThread());
 
-  // Tell the event thread it will soon be time to shut down. This gives
-  // us assurance the thread will stop in case the SND_SEQ_EVENT_CLIENT_EXIT
-  // message is lost.
+  // Tell tasks running on TaskRunners it will soon be time to shut down. This
+  // gives us assurance a task running on kEventTaskRunner will stop in case the
+  // SND_SEQ_EVENT_CLIENT_EXIT message is lost.
   {
-    base::AutoLock lock(shutdown_lock_);
-    event_thread_shutdown_ = true;
+    base::AutoLock lock(g_instance_id_lock.Get());
+    CHECK_EQ(instance_id_, g_active_instance_id);
+    g_active_instance_id = kInvalidInstanceId;
   }
 
-  // Stop the send thread.
-  send_thread_.Stop();
+  // Ensure that no tasks run on kSendTaskRunner.
+  base::AutoLock send_runner_lock(g_send_task_lock.Get());
 
   // Close the out client. This will trigger the event thread to stop,
   // because of SND_SEQ_EVENT_CLIENT_EXIT.
   out_client_.reset();
 
-  // Wait for the event thread to stop.
-  event_thread_.Stop();
+  // Ensure that no tasks run on kEventTaskRunner.
+  base::AutoLock event_runner_lock(g_event_task_lock.Get());
 
   // Destruct the other stuff we initialized in StartInitialization().
   udev_monitor_.reset();
@@ -339,15 +360,13 @@ void MidiManagerAlsa::DispatchSendMidiData(MidiManagerClient* client,
     delay = std::max(time_to_send - base::TimeTicks::Now(), base::TimeDelta());
   }
 
-  send_thread_.task_runner()->PostDelayedTask(
-      FROM_HERE, base::Bind(&MidiManagerAlsa::SendMidiData,
-                            base::Unretained(this), port_index, data),
-      delay);
-
-  // Acknowledge send.
-  send_thread_.task_runner()->PostTask(
-      FROM_HERE, base::Bind(&MidiManagerAlsa::AccumulateMidiBytesSent,
-                            base::Unretained(this), client, data.size()));
+  service()
+      ->GetTaskRunner(kSendTaskRunner)
+      ->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&MidiManagerAlsa::SendMidiData, base::Unretained(this),
+                     instance_id_, client, port_index, data),
+          delay);
 }
 
 MidiManagerAlsa::MidiPort::Id::Id() = default;
@@ -864,9 +883,22 @@ std::string MidiManagerAlsa::AlsaCard::ExtractManufacturerString(
   return "";
 }
 
-void MidiManagerAlsa::SendMidiData(uint32_t port_index,
+void MidiManagerAlsa::SendMidiData(int instance_id,
+                                   MidiManagerClient* client,
+                                   uint32_t port_index,
                                    const std::vector<uint8_t>& data) {
-  DCHECK(send_thread_.task_runner()->BelongsToCurrentThread());
+  DCHECK(service()->GetTaskRunner(kSendTaskRunner)->BelongsToCurrentThread());
+
+  // Obtain the lock so that the instance could not be destructed while this
+  // method is running on the kSendTaskRunner.
+  base::AutoLock lock(g_send_task_lock.Get());

[yhirano, 2017/02/08 10:04:37]

I think moving this below the instance id check is better.

[Takashi Toyoshima, 2017/02/08 11:08:55]

If this lock is created after the instance id check, we would overlook a race
here.

1. Send: ID check pass
2. I/O : Finalize() is running in parallel
3. I/O : Finalize() finishes
4. Send: g_send_task_lock is obtained

If we keep g_send_task_lock obtained as this patch, we can keep Finalize()
unfinished until this task finishes.

+  {
+    // Check if Finalize() already runs. After this check, we can access |this|
+    // safely on the kEventTaskRunner.
+    base::AutoLock instance_id_lock(g_instance_id_lock.Get());
+    if (instance_id != g_active_instance_id)
+      return;
+  }
 
   snd_midi_event_t* encoder;
   snd_midi_event_new(kSendBufferSize, &encoder);
@@ -886,15 +918,23 @@ void MidiManagerAlsa::SendMidiData(uint32_t port_index,
     }
   }
   snd_midi_event_free(encoder);
-}
 
-void MidiManagerAlsa::ScheduleEventLoop() {
-  event_thread_.task_runner()->PostTask(
-      FROM_HERE,
-      base::Bind(&MidiManagerAlsa::EventLoop, base::Unretained(this)));
+  // Acknowledge send.
+  AccumulateMidiBytesSent(client, data.size());
 }
 
-void MidiManagerAlsa::EventLoop() {
+void MidiManagerAlsa::EventLoop(int instance_id) {
+  // Obtain the lock so that the instance could not be destructed while this
+  // method is running on the kEventTaskRunner.
+  base::AutoLock lock(g_event_task_lock.Get());

[yhirano, 2017/02/08 10:04:37]

I think moving this below the instance id check is better.

[Takashi Toyoshima, 2017/02/08 11:08:55]

same

+  {
+    // Check if Finalize() already runs. After this check, we can access |this|
+    // safely on the kEventTaskRunner.
+    base::AutoLock instance_id_lock(g_instance_id_lock.Get());
+    if (instance_id != g_active_instance_id)
+      return;
+  }
+
   bool loop_again = true;
 
   struct pollfd pfd[2];
@@ -922,9 +962,6 @@ void MidiManagerAlsa::EventLoop() {
           VLOG(1) << "snd_seq_event_input detected buffer overrun";
           // We've lost events: check another way to see if we need to shut
           // down.
-          base::AutoLock lock(shutdown_lock_);
-          if (event_thread_shutdown_)
-            loop_again = false;
         } else if (err == -EAGAIN) {
           // We've read all the data.
         } else if (err < 0) {
@@ -973,8 +1010,12 @@ void MidiManagerAlsa::EventLoop() {
   }
 
   // Do again.
-  if (loop_again)
-    ScheduleEventLoop();
+  if (loop_again) {
+    service()
+        ->GetTaskRunner(kEventTaskRunner)
+        ->PostTask(FROM_HERE, base::Bind(&MidiManagerAlsa::EventLoop,
+                                         base::Unretained(this), instance_id));
+  }
 }
 
 void MidiManagerAlsa::ProcessSingleEvent(snd_seq_event_t* event,
@@ -1401,8 +1442,8 @@ bool MidiManagerAlsa::Subscribe(uint32_t port_index,
   return true;
 }
 
-MidiManager* MidiManager::Create() {
-  return new MidiManagerAlsa();
+MidiManager* MidiManager::Create(MidiService* service) {
+  return new MidiManagerAlsa(service);
 }
 
 }  // namespace midi