Automated g4 rollback of changelist 150936297.

*** Reason for rollback ***

Try adding this flag again, this time using the pre-existing requireStub() method that other methods are using to avoid trying to load classes too early.

That should avoid the race condition that caused the original rollback.

*** Original change description ***

Automated g4 rollback of changelist 149961619.

*** Reason for rollback ***

Causes deadlock. b/36526050

*** Original change description ***

Reimplement cl/146413726 to do the borg checking in Java.

  This change was needed due to a circular dependency introducted by the last
  change.  See the comments in cl/146413726 for details.

  Unfortunately, this change requires adding a method to the JNI bridge.  This
  requires many changes and complicates the implementation.  Indeed most of the
  c...

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=159229602

Author: mattwach

src/agent/bridge.h

@@ -76,6 +76,12 @@ class Bridge {
 
   // Approves the breakpoint for a global rollout.
   virtual bool ApproveBreakpointCanary(const string& breakpoint_id) = 0;
+
+  // Tries to determine if the debugger is enabled.  If the debugger is enabled,
+  // sets is_enabled to true and returns true.  If the debugger is disabled,
+  // sets is_enabled to false and returns true.  If the method can not determine
+  // status, sets is_enabled to false and returns false.
+  virtual bool IsEnabled(bool* is_enabled) = 0;
 };
 
 }  // namespace cdbg

src/agent/internals/src/main/java/com/google/devtools/cdbg/debuglets/java/GcpHubClient.java

@@ -508,6 +508,14 @@ public void approveBreakpointCanary(String breakpointId) throws Exception {
     throw new UnsupportedOperationException("GcpHubClient#approveBreakpointCanary not implemented");
   }
 
+  /**
+   * If there is any reason to not enable the debugger, return false here.
+   */
+  @Override
+  public boolean isEnabled() {
+    return true;
+  }
+
   @Override
   public void shutdown() {
     metadata.shutdown();

src/agent/internals/src/main/java/com/google/devtools/cdbg/debuglets/java/HubClient.java

@@ -142,7 +142,12 @@ void transmitBreakpointUpdate(String format, String breakpointId, byte[] breakpo
    * @throws Exception if the backend could not be notified or on failure on the backend side.
    */
   void approveBreakpointCanary(String breakpointId) throws Exception;
-  
+
+  /**
+   * Returns false if this agent should not be enabled.
+   */
+  boolean isEnabled() throws Exception;
+
   /**
    * Asynchronously aborts all pending requests and blocks any future network connections.
    */

src/agent/jni_bridge.cc

@@ -220,6 +220,18 @@ bool JniBridge::ApproveBreakpointCanary(const string& breakpoint_id) {
 }
 
 
+bool JniBridge::IsEnabled(bool* is_enabled) {
+  auto rc = jniproxy::HubClient()->isEnabled(jni_hub_.get());
+  if (rc.HasException()) {
+    *is_enabled = false;
+    return false;
+  }
+
+  *is_enabled = rc.GetData();
+  return true;
+}
+
+
 void JniBridge::Shutdown() {
   MutexLock lock(&mu_);
 

src/agent/jni_bridge.h

@@ -69,6 +69,13 @@ class JniBridge : public Bridge {
 
   bool ApproveBreakpointCanary(const string& breakpoint_id) override;
 
+  // TODO(mattwach): This method might be better put into it's own interface.
+  // The reason it's not done now is that creating a new interface involves
+  // a lot of change and boilerplate.  It would seems reasonable to see the need
+  // for an interface expanded a bit before putting the definitions and
+  // rules in place.
+  bool IsEnabled(bool* is_enabled) override;
+
  private:
   // Callback to create an instance of Java class implementing
   // the com.google.devtools.cdbg.debuglets.java.HubClient interface.

src/agent/worker.cc

@@ -38,7 +38,7 @@ DEFINE_FLAG(
 namespace devtools {
 namespace cdbg {
 
-int g_register_debuggee_attempts = 0;
+int g_is_enabled_attempts = 0;
 
 Worker::Worker(
     Provider* provider,
@@ -146,6 +146,19 @@ void Worker::MainThreadProc() {
     return;  // Signal to stop the main debugger thread.
   }
 
+  bool is_enabled = false;
+  while (!is_unloading_ && !bridge_->IsEnabled(&is_enabled)) {
+    // Wait for classes to load.
+    ++g_is_enabled_attempts;
+    provider_->OnIdle();
+    main_thread_event_->Wait(100);  // Wait for 100ms to lower CPU usage
+  }
+
+  if (!is_enabled) {
+    LOG(WARNING) << "The debugger is disabled on this process.";
+    return;
+  }
+
   while (!is_unloading_) {
     // Register debuggee if not registered or if previous call to list active
     // breakpoints failed.
@@ -202,7 +215,6 @@ void Worker::TransmissionThreadProc() {
 void Worker::RegisterDebuggee() {
   bool new_is_enabled = false;
   is_registered_ = bridge_->RegisterDebuggee(&new_is_enabled);
-  ++g_register_debuggee_attempts;
 
   if (is_registered_) {
     provider_->EnableDebugger(new_is_enabled);

src/codegen/config.json

@@ -288,6 +288,9 @@
         },
         {
           "methodName": "shutdown"
+        },
+        {
+          "methodName": "isEnabled"
         }
       ],
       "nativeNamespace": "jniproxy"