Web MIDI: introduce pending client count limit to start sessions

media/midi/midi_manager.cc

 // Copyright (c) 2013 The Chromium 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 "media/midi/midi_manager.h"
 
 #include "base/debug/trace_event.h"
 
 namespace media {
 
-#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(USE_ALSA) && \
-    !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
-MidiManager* MidiManager::Create() {
-  return new MidiManager;
-}
-#endif
-
 MidiManager::MidiManager()
     : initialized_(false),
       result_(MIDI_NOT_SUPPORTED) {
 }
 
 MidiManager::~MidiManager() {
 }
 
+#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(USE_ALSA) && \
+    !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+MidiManager* MidiManager::Create() {
+  return new MidiManager;
+}
+#endif
+
 void MidiManager::StartSession(MidiManagerClient* client, int client_id) {
   bool session_is_ready;
   bool session_needs_initialization = false;
+  bool too_many_pending_clients_exist = false;
 
   {
     base::AutoLock auto_lock(clients_lock_);
     session_is_ready = initialized_;
     if (!session_is_ready) {
-      // Call StartInitialization() only for the first request.
-      session_needs_initialization = pending_clients_.empty();
-      pending_clients_.insert(
-          std::pair<int, MidiManagerClient*>(client_id, client));
+      // Do not accept a new request if the pending client list contains too
+      // many clients.
+      too_many_pending_clients_exist =
+          pending_clients_.size() >= kMaxPendingClientCount;
+
+      if (!too_many_pending_clients_exist) {
+        // Call StartInitialization() only for the first request.
+        session_needs_initialization = pending_clients_.empty();
+        pending_clients_.insert(
+            std::pair<int, MidiManagerClient*>(client_id, client));
+      }
     }
   }
 
   // Lazily initialize the MIDI back-end.
   if (!session_is_ready) {
     if (session_needs_initialization) {
       TRACE_EVENT0("midi", "MidiManager::StartInitialization");
       StartInitialization();
     }
+    if (too_many_pending_clients_exist) {

[yukawa, 2014/05/04 22:44:54]

|too_many_pending_clients_exist| isn't guarded by |clients_lock_|. Is there any
race condition here? or it's acceptable?
Sorry if the issue already existed in previous CLs.

[Takashi Toyoshima, 2014/05/04 23:28:44]

We do not want to hold |clients_lock_| to invoke virtual functions and callbacks
here. E.g., StartInitialization() may calls CompleteInitialization() internally,
and it causes a dead-lock to get the same lock on the same thread.

I think it's ok to reject the request that makes
|too_many_pending_clients_exist| true even if a previous pending request is
finished just now in a kind of race condition. Anyway, having over 100 requests
must be too curious situation and must be under a malicious attack.

[yukawa, 2014/05/04 23:45:55]

Got it. Thanks.

+      // Return an error immediately if there are too many requests.
+      client->CompleteStartSession(client_id, MIDI_INITIALIZATION_ERROR);
+      return;
+    }
     // CompleteInitialization() will be called asynchronously when platform
     // dependent initialization is finished.
     return;
   }
 
   // Platform dependent initialization was already finished for previously
   // initialized clients.
   MidiResult result;
   {
     base::AutoLock auto_lock(clients_lock_);
@@ skipping 55 matching lines @@
     const uint8* data,
     size_t length,
     double timestamp) {
   base::AutoLock auto_lock(clients_lock_);
 
   for (ClientList::iterator i = clients_.begin(); i != clients_.end(); ++i)
     (*i)->ReceiveMidiData(port_index, data, length, timestamp);
 }
 
 }  // namespace media