sync: Gracefully handle very early shutdown

sync/engine/net/server_connection_manager.h

 // Copyright 2012 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.
 
 #ifndef SYNC_ENGINE_NET_SERVER_CONNECTION_MANAGER_H_
 #define SYNC_ENGINE_NET_SERVER_CONNECTION_MANAGER_H_
 
 #include <iosfwd>
 #include <string>
 
-#include "base/atomicops.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/strings/string_util.h"
-#include "base/synchronization/lock.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/threading/thread_checker.h"
 #include "sync/base/sync_export.h"
+#include "sync/internal_api/public/base/cancelation_observer.h"
 #include "sync/syncable/syncable_id.h"
 
 namespace sync_pb {
 class ClientToServerMessage;
 }
 
 namespace syncer {
 
+class CancelationSignal;
+
 namespace syncable {
 class Directory;
 }
 
 static const int32 kUnsetResponseCode = -1;
 static const int32 kUnsetContentLength = -1;
 static const int32 kUnsetPayloadLength = -1;
 
 // HttpResponse gathers the relevant output properties of an HTTP request.
 // Depending on the value of the server_status code, response_code, and
@@ skipping 139 matching lines @@
     ServerConnectionManager* scm_;
 
    private:
     int ReadResponse(void* buffer, int length);
     int ReadResponse(std::string* buffer, int length);
   };
 
   ServerConnectionManager(const std::string& server,
                           int port,
                           bool use_ssl,
-                          bool use_oauth2_token);
+                          bool use_oauth2_token,
+                          CancelationSignal* const cancelation_signal);

[tim (not reviewing), 2013/09/06 21:47:34]

I think you can omit the *const here and keep it in the impl only?  It's weird
to see here since it's useless to the caller, right? 

[rlarocque, 2013/09/06 22:42:53]

Yep, this is the result of a regex gone wrong.  I thought I had fixed all
instances, but I guess I missed this one.

 
   virtual ~ServerConnectionManager();
 
   // POSTS buffer_in and reads a response into buffer_out. Uses our currently
   // set auth token in our headers.
   //
   // Returns true if executed successfully.
   virtual bool PostBufferWithCachedAuth(PostBufferParams* params,
                                         ScopedServerStatusWatcher* watcher);
 
   void AddListener(ServerConnectionEventListener* listener);
   void RemoveListener(ServerConnectionEventListener* listener);
 
   inline HttpResponse::ServerConnectionCode server_status() const {
     DCHECK(thread_checker_.CalledOnValidThread());
     return server_status_;
   }
 
   const std::string client_id() const { return client_id_; }
 
   // Returns the current server parameters in server_url, port and use_ssl.
   void GetServerParameters(std::string* server_url,
                            int* port,
                            bool* use_ssl) const;
 
   std::string GetServerHost() const;
 
   // Factory method to create an Connection object we can use for
   // communication with the server.
-  virtual Connection* MakeConnection();
-
-  // Aborts any active HTTP POST request.
-  // We expect this to get called on a different thread than the valid
-  // ThreadChecker thread, as we want to kill any pending http traffic without
-  // having to wait for the request to complete.
-  void TerminateAllIO();
+  virtual scoped_ptr<Connection> MakeConnection();
 
   void set_client_id(const std::string& client_id) {
     DCHECK(thread_checker_.CalledOnValidThread());
     DCHECK(client_id_.empty());
     client_id_.assign(client_id);
   }
 
   // Sets a new auth token and time.
   bool SetAuthToken(const std::string& auth_token);
 
@@ skipping 32 matching lines @@
   // An internal helper to clear our auth_token_ and cache the old version
   // in |previously_invalidated_token_| to shelter us from retrying with a
   // known bad token.
   void InvalidateAndClearAuthToken();
 
   // Helper to check terminated flags and build a Connection object, installing
   // it as the |active_connection_|.  If this ServerConnectionManager has been
   // terminated, this will return NULL.
   Connection* MakeActiveConnection();
 
-  // Called by Connection objects as they are destroyed to allow the
-  // ServerConnectionManager to cleanup active connections.
-  void OnConnectionDestroyed(Connection* connection);
-
   // The sync_server_ is the server that requests will be made to.
   std::string sync_server_;
 
   // The sync_server_port_ is the port that HTTP requests will be made on.
   int sync_server_port_;
 
   // The unique id of the user's client.
   std::string client_id_;
 
   // Indicates whether or not requests should be made using HTTPS.
@@ skipping 12 matching lines @@
 
   // The previous auth token that is invalid now.
   std::string previously_invalidated_token;
 
   ObserverList<ServerConnectionEventListener> listeners_;
 
   HttpResponse::ServerConnectionCode server_status_;
 
   base::ThreadChecker thread_checker_;
 
-  // Protects all variables below to allow bailing out of active connections.
-  base::Lock terminate_connection_lock_;
-
-  // If true, we've been told to terminate IO and expect to be destroyed
-  // shortly.  No future network requests will be made.
-  bool terminated_;
-
-  // A non-owning pointer to any active http connection, so that we can abort
-  // it if necessary.
-  Connection* active_connection_;
+  CancelationSignal* const cancelation_signal_;
 
  private:
   friend class Connection;
   friend class ScopedServerStatusWatcher;
 
-  // A class to help deal with cleaning up active Connection objects when (for
-  // ex) multiple early-exits are present in some scope. ScopedConnectionHelper
-  // informs the ServerConnectionManager before the Connection object it takes
-  // ownership of is destroyed.
-  class ScopedConnectionHelper {
+  // A class to help manage the active connection.  It handles the registration
+  // and unregistration with the CancelationSignal.  It also takes ownership of
+  // the connection and will delete it if the abort signal was sent early or
+  // when this class goes out of scope.
+  class ScopedConnectionHelper : public CancelationObserver {
    public:
-    // |manager| must outlive this. Takes ownership of |connection|.
-    ScopedConnectionHelper(ServerConnectionManager* manager,
-                           Connection* connection);
-    ~ScopedConnectionHelper();
+    ScopedConnectionHelper(CancelationSignal* cancelation_signal,
+                           scoped_ptr<Connection> connection);
+    virtual ~ScopedConnectionHelper();
     Connection* get();
+
+    // Called from a different thread when the CancelationObserver's
+    // RequestStop() is called and this class has been registered as a handler.
+    //
+    // Marked final because there's no way to safely override it.  See comment
+    // in this class' destructor.
+    virtual void OnStopRequested() OVERRIDE FINAL;
+
    private:
-    ServerConnectionManager* manager_;
+    CancelationSignal* const cancelation_signal_;
     scoped_ptr<Connection> connection_;
+
     DISALLOW_COPY_AND_ASSIGN(ScopedConnectionHelper);
   };
 
   void NotifyStatusChanged();
 
   DISALLOW_COPY_AND_ASSIGN(ServerConnectionManager);
 };
 
 std::ostream& operator<<(std::ostream& s, const struct HttpResponse& hr);
 
 }  // namespace syncer
 
 #endif  // SYNC_ENGINE_NET_SERVER_CONNECTION_MANAGER_H_