From 3d3e066a8d2841456bde28be3c0e3d9a0f836cad Mon Sep 17 00:00:00 2001
From: James McTavish <mctavish@google.com>
Date: Fri, 17 Feb 2023 09:31:47 -0500
Subject: [PATCH 1/6] fix: handle negative line number deltas (#73)

In Python 3.6-3.9, unlike in Python 2, co_lnotab's line number offsets can be negative and are represented by a signed integer. The Debugger agent had been using an unsigned integer which prevented it from being able to set breakpoints on lines of code in a code object after the line table had a negative offset.

This PR also enables most of the previously disabled tests now that they work on all supported versions of Python. A couple still remain disabled and will be fixed later on.
---
 src/googleclouddebugger/module_explorer.py    |  5 +-
 src/googleclouddebugger/python_util.cc        |  2 +-
 ...n_test_disabled.py => integration_test.py} | 55 +++++++++----------
 ...st_disabled.py => module_explorer_test.py} |  2 -
 ..._disabled.py => python_breakpoint_test.py} | 45 +++++++++------
 5 files changed, 59 insertions(+), 50 deletions(-)
 rename tests/py/{integration_test_disabled.py => integration_test.py} (94%)
 rename tests/py/{module_explorer_test_disabled.py => module_explorer_test.py} (99%)
 rename tests/py/{python_breakpoint_test_disabled.py => python_breakpoint_test.py} (95%)

diff --git a/src/googleclouddebugger/module_explorer.py b/src/googleclouddebugger/module_explorer.py
index 99829df..ac62ce4 100644
--- a/src/googleclouddebugger/module_explorer.py
+++ b/src/googleclouddebugger/module_explorer.py
@@ -62,7 +62,7 @@ def GetCodeObjectAtLine(module, line):
         prev_line = max(prev_line, co_line_number)
       elif co_line_number > line:
         next_line = min(next_line, co_line_number)
-        break
+        # Continue because line numbers may not be sequential.
 
   prev_line = None if prev_line == 0 else prev_line
   next_line = None if next_line == sys.maxsize else next_line
@@ -87,6 +87,9 @@ def _GetLineNumbers(code_object):
     line_incrs = code_object.co_lnotab[1::2]
     current_line = code_object.co_firstlineno
     for line_incr in line_incrs:
+      if line_incr >= 0x80:
+        # line_incrs is an array of 8-bit signed integers
+        line_incr -= 0x100
       current_line += line_incr
       yield current_line
   else:
diff --git a/src/googleclouddebugger/python_util.cc b/src/googleclouddebugger/python_util.cc
index e28a142..bc03bfc 100644
--- a/src/googleclouddebugger/python_util.cc
+++ b/src/googleclouddebugger/python_util.cc
@@ -79,7 +79,7 @@ bool CodeObjectLinesEnumerator::Next() {
 
   while (true) {
     offset_ += next_entry_[0];
-    line_number_ += next_entry_[1];
+    line_number_ += static_cast<int8_t>(next_entry_[1]);
 
     bool stop = ((next_entry_[0] != 0xFF) || (next_entry_[1] != 0)) &&
                 ((next_entry_[0] != 0) || (next_entry_[1] != 0xFF));
diff --git a/tests/py/integration_test_disabled.py b/tests/py/integration_test.py
similarity index 94%
rename from tests/py/integration_test_disabled.py
rename to tests/py/integration_test.py
index 12321c4..39067b0 100644
--- a/tests/py/integration_test_disabled.py
+++ b/tests/py/integration_test.py
@@ -1,7 +1,5 @@
 """Complete tests of the debugger mocking the backend."""
 
-# TODO: Get this test to work well all supported versions of python.
-
 from datetime import datetime
 from datetime import timedelta
 import functools
@@ -20,7 +18,7 @@
 import google.auth
 from absl.testing import absltest
 
-from googleclouddebugger import capture_collector
+from googleclouddebugger import collector
 from googleclouddebugger import labels
 import python_test_util
 
@@ -98,7 +96,7 @@ def GetVal():
       cdbg.enable()
 
       # Increase the polling rate to speed up the test.
-      cdbg._hub_client.min_interval_sec = 0.001  # Poll every 1 ms
+      cdbg.gcp_hub_client.min_interval_sec = 0.001  # Poll every 1 ms
 
     def SetBreakpoint(self, tag, template=None):
       """Sets a new breakpoint in this source file.
@@ -214,7 +212,6 @@ def execute(self):  # pylint: disable=invalid-name
 
       return FakeBreakpointUpdateCommand(self._incoming_breakpoint_updates)
 
-
 # We only need to attach the debugger exactly once. The IntegrationTest class
 # is created for each test case, so we need to keep this state global.
 
@@ -226,7 +223,7 @@ def _FakeLog(self, message, extra=None):
 
   def setUp(self):
     self._info_log = []
-    capture_collector.log_info_message = self._FakeLog
+    collector.log_info_message = self._FakeLog
 
   def tearDown(self):
     IntegrationTest._hub.SetActiveBreakpoints([])
@@ -281,8 +278,8 @@ def testExistingLabelsPriority(self):
     def Trigger():
       print('Breakpoint trigger with labels')  # BPTAG: EXISTING_LABELS_PRIORITY
 
-    current_labels_collector = capture_collector.breakpoint_labels_collector
-    capture_collector.breakpoint_labels_collector = \
+    current_labels_collector = collector.breakpoint_labels_collector
+    collector.breakpoint_labels_collector = \
         lambda: {'label_1': 'value_1', 'label_2': 'value_2'}
 
     IntegrationTest._hub.SetBreakpoint(
@@ -294,7 +291,7 @@ def Trigger():
 
     Trigger()
 
-    capture_collector.breakpoint_labels_collector = current_labels_collector
+    collector.breakpoint_labels_collector = current_labels_collector
 
     # In this case, label_1 was in both the agent and the pre existing labels,
     # the pre existing value of value_foobar should be preserved.
@@ -313,14 +310,14 @@ def Trigger():
       print('Breakpoint trigger req id label')  # BPTAG: REQUEST_LOG_ID_LABEL
 
     current_request_log_id_collector = \
-      capture_collector.request_log_id_collector
-    capture_collector.request_log_id_collector = lambda: 'foo_bar_id'
+      collector.request_log_id_collector
+    collector.request_log_id_collector = lambda: 'foo_bar_id'
 
     IntegrationTest._hub.SetBreakpoint('REQUEST_LOG_ID_LABEL')
 
     Trigger()
 
-    capture_collector.request_log_id_collector = \
+    collector.request_log_id_collector = \
         current_request_log_id_collector
 
     result = IntegrationTest._hub.GetNextResult()
@@ -559,23 +556,25 @@ def Method(a):
         }, python_test_util.PackFrameVariable(result, 'x', frame=1))
     return x
 
-  def testRecursion(self):
-
-    def RecursiveMethod(i):
-      if i == 0:
-        return 0  # BPTAG: RECURSION
-      return RecursiveMethod(i - 1)
-
-    IntegrationTest._hub.SetBreakpoint('RECURSION')
-    RecursiveMethod(5)
-    result = IntegrationTest._hub.GetNextResult()
 
-    for frame in range(5):
-      self.assertEqual({
-          'name': 'i',
-          'value': str(frame),
-          'type': 'int'
-      }, python_test_util.PackFrameVariable(result, 'i', frame, 'arguments'))
+# FIXME: Broken in Python 3.10
+#  def testRecursion(self):
+#
+#    def RecursiveMethod(i):
+#      if i == 0:
+#        return 0  # BPTAG: RECURSION
+#      return RecursiveMethod(i - 1)
+#
+#    IntegrationTest._hub.SetBreakpoint('RECURSION')
+#    RecursiveMethod(5)
+#    result = IntegrationTest._hub.GetNextResult()
+#
+#    for frame in range(5):
+#      self.assertEqual({
+#          'name': 'i',
+#          'value': str(frame),
+#          'type': 'int'
+#      }, python_test_util.PackFrameVariable(result, 'i', frame, 'arguments'))
 
   def testWatchedExpressions(self):
 
diff --git a/tests/py/module_explorer_test_disabled.py b/tests/py/module_explorer_test.py
similarity index 99%
rename from tests/py/module_explorer_test_disabled.py
rename to tests/py/module_explorer_test.py
index b7542e1..4e1a42c 100644
--- a/tests/py/module_explorer_test_disabled.py
+++ b/tests/py/module_explorer_test.py
@@ -1,7 +1,5 @@
 """Unit test for module_explorer module."""
 
-# TODO: Get this test to run properly on all supported versions of Python
-
 import dis
 import inspect
 import os
diff --git a/tests/py/python_breakpoint_test_disabled.py b/tests/py/python_breakpoint_test.py
similarity index 95%
rename from tests/py/python_breakpoint_test_disabled.py
rename to tests/py/python_breakpoint_test.py
index a337d12..153a2ba 100644
--- a/tests/py/python_breakpoint_test_disabled.py
+++ b/tests/py/python_breakpoint_test.py
@@ -1,7 +1,5 @@
 """Unit test for python_breakpoint module."""
 
-# TODO: Get this test to work with all supported versions of Python.
-
 from datetime import datetime
 from datetime import timedelta
 import inspect
@@ -12,7 +10,7 @@
 from absl.testing import absltest
 
 from googleclouddebugger import cdbg_native as native
-from googleclouddebugger import imphook2
+from googleclouddebugger import imphook
 from googleclouddebugger import python_breakpoint
 import python_test_util
 
@@ -102,7 +100,7 @@ def testDeferredBreakpoint(self):
                      self._update_queue[0]['stackFrames'][0]['function'])
     self.assertTrue(self._update_queue[0]['isFinalState'])
 
-    self.assertEmpty(imphook2._import_callbacks)
+    self.assertEmpty(imphook._import_callbacks)
 
   # Old module search algorithm rejects multiple matches. This test verifies
   # that the new module search algorithm searches sys.path sequentially, and
@@ -142,7 +140,7 @@ def testSearchUsingSysPathOrder(self):
     self.assertEqual(
         '2', self._update_queue[0]['stackFrames'][0]['locals'][0]['value'])
 
-    self.assertEmpty(imphook2._import_callbacks)
+    self.assertEmpty(imphook._import_callbacks)
 
   # Old module search algorithm rejects multiple matches. This test verifies
   # that when the new module search cannot find any match in sys.path, it
@@ -183,7 +181,7 @@ def testMultipleDeferredMatches(self):
     self.assertEqual(
         '1', self._update_queue[0]['stackFrames'][0]['locals'][0]['value'])
 
-    self.assertEmpty(imphook2._import_callbacks)
+    self.assertEmpty(imphook._import_callbacks)
 
   def testNeverLoadedBreakpoint(self):
     open(os.path.join(self._test_package_dir, 'never_print.py'), 'w').close()
@@ -223,7 +221,7 @@ def testDeferredNoCodeAtLine(self):
     params = desc['parameters']
     self.assertIn('defer_empty.py', params[1])
     self.assertEqual(params[0], '10')
-    self.assertEmpty(imphook2._import_callbacks)
+    self.assertEmpty(imphook._import_callbacks)
 
   def testDeferredBreakpointCancelled(self):
     open(os.path.join(self._test_package_dir, 'defer_cancel.py'), 'w').close()
@@ -236,7 +234,7 @@ def testDeferredBreakpointCancelled(self):
     breakpoint.Clear()
 
     self.assertFalse(self._completed)
-    self.assertEmpty(imphook2._import_callbacks)
+    self.assertEmpty(imphook._import_callbacks)
     unused_no_code_line_above = 0  # BPTAG: NO_CODE_LINE_ABOVE
 
   # BPTAG: NO_CODE_LINE
@@ -345,7 +343,7 @@ def testNonRootInitFile(self):
       self.assertEqual('DoPrint',
                        self._update_queue[0]['stackFrames'][0]['function'])
 
-      self.assertEmpty(imphook2._import_callbacks)
+      self.assertEmpty(imphook._import_callbacks)
       self._update_queue = []
 
   def testBreakpointInLoadedPackageFile(self):
@@ -414,15 +412,26 @@ def testInvalidCondition(self):
     self.assertEqual(set(['BP_ID']), self._completed)
     self.assertLen(self._update_queue, 1)
     self.assertTrue(self._update_queue[0]['isFinalState'])
-    self.assertEqual(
-        {
-            'isError': True,
-            'refersTo': 'BREAKPOINT_CONDITION',
-            'description': {
-                'format': 'Expression could not be compiled: $0',
-                'parameters': ['unexpected EOF while parsing']
-            }
-        }, self._update_queue[0]['status'])
+    if sys.version_info.minor < 10:
+      self.assertEqual(
+          {
+              'isError': True,
+              'refersTo': 'BREAKPOINT_CONDITION',
+              'description': {
+                  'format': 'Expression could not be compiled: $0',
+                  'parameters': ['unexpected EOF while parsing']
+              }
+          }, self._update_queue[0]['status'])
+    else:
+      self.assertEqual(
+          {
+              'isError': True,
+              'refersTo': 'BREAKPOINT_CONDITION',
+              'description': {
+                  'format': 'Expression could not be compiled: $0',
+                  'parameters': ['invalid syntax']
+              }
+          }, self._update_queue[0]['status'])
 
   def testHit(self):
     breakpoint = python_breakpoint.PythonBreakpoint(self._template, self, self,

From 7d09b9ab99410d8ffe7218a21ea9386eb51d34f2 Mon Sep 17 00:00:00 2001
From: jasonborg <48138260+jasonborg@users.noreply.github.com>
Date: Tue, 21 Feb 2023 19:20:15 +0000
Subject: [PATCH 2/6] fix: Breakpoint Expiry in Firebase Backend Version (#74)

- The firebase backend client code now calls on_idle periodically
- The breakpoint expiration code now checks for the `createTimeUnixMsec` breakpoint field

Fixes #72
---
 src/googleclouddebugger/firebase_client.py   |  6 ++++
 src/googleclouddebugger/python_breakpoint.py | 36 ++++++++++++++++----
 tests/py/python_breakpoint_test.py           | 28 +++++++++++++++
 tests/py/python_test_util.py                 |  4 +++
 4 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/src/googleclouddebugger/firebase_client.py b/src/googleclouddebugger/firebase_client.py
index be59a3c..474211a 100644
--- a/src/googleclouddebugger/firebase_client.py
+++ b/src/googleclouddebugger/firebase_client.py
@@ -311,6 +311,12 @@ def _MainThreadProc(self):
 
     self._StartMarkActiveTimer()
 
+    while not self._shutdown:
+      if self.on_idle is not None:
+        self.on_idle()
+
+      time.sleep(1)
+
   def _TransmissionThreadProc(self):
     """Entry point for the transmission worker thread."""
 
diff --git a/src/googleclouddebugger/python_breakpoint.py b/src/googleclouddebugger/python_breakpoint.py
index 1339ebe..c65b95d 100644
--- a/src/googleclouddebugger/python_breakpoint.py
+++ b/src/googleclouddebugger/python_breakpoint.py
@@ -252,16 +252,40 @@ def GetBreakpointId(self):
     return self.definition['id']
 
   def GetExpirationTime(self):
-    """Computes the timestamp at which this breakpoint will expire."""
-    # TODO: Move this to a common method.
-    if '.' not in self.definition['createTime']:
+    """Computes the timestamp at which this breakpoint will expire.
+
+    If no creation time can be found an expiration time in the past will be
+    used.
+    """
+    return self.GetCreateTime() + self.expiration_period
+
+  def GetCreateTime(self):
+    """Retrieves the creation time of this breakpoint.
+
+    If no creation time can be found a creation time in the past will be used.
+    """
+    if 'createTime' in self.definition:
+      return self.GetTimeFromRfc3339Str(self.definition['createTime'])
+    else:
+      return self.GetTimeFromUnixMsec(
+          self.definition.get('createTimeUnixMsec', 0))
+
+  def GetTimeFromRfc3339Str(self, rfc3339_str):
+    if '.' not in rfc3339_str:
       fmt = '%Y-%m-%dT%H:%M:%S%Z'
     else:
       fmt = '%Y-%m-%dT%H:%M:%S.%f%Z'
 
-    create_datetime = datetime.strptime(
-        self.definition['createTime'].replace('Z', 'UTC'), fmt)
-    return create_datetime + self.expiration_period
+    return datetime.strptime(rfc3339_str.replace('Z', 'UTC'), fmt)
+
+  def GetTimeFromUnixMsec(self, unix_msec):
+    try:
+      return datetime.fromtimestamp(unix_msec / 1000)
+    except (TypeError, ValueError, OSError, OverflowError) as e:
+      native.LogWarning(
+        'Unexpected error (%s) occured processing unix_msec %s, breakpoint: %s'
+        % (repr(e), str(unix_msec), self.GetBreakpointId()))
+      return datetime.fromtimestamp(0)
 
   def ExpireBreakpoint(self):
     """Expires this breakpoint."""
diff --git a/tests/py/python_breakpoint_test.py b/tests/py/python_breakpoint_test.py
index 153a2ba..6aff9c4 100644
--- a/tests/py/python_breakpoint_test.py
+++ b/tests/py/python_breakpoint_test.py
@@ -457,6 +457,22 @@ def testHitNewTimestamp(self):
     self.assertGreater(len(self._update_queue[0]['stackFrames']), 3)
     self.assertTrue(self._update_queue[0]['isFinalState'])
 
+  def testHitTimestampUnixMsec(self):
+    # Using the Snapshot Debugger (Firebase backend) version of creation time
+    self._template.pop('createTime', None);
+    self._template[
+      'createTimeUnixMsec'] = python_test_util.DateTimeToUnixMsec(
+        self._base_time)
+
+    breakpoint = python_breakpoint.PythonBreakpoint(self._template, self, self,
+                                                    None)
+    breakpoint._BreakpointEvent(native.BREAKPOINT_EVENT_HIT,
+                                inspect.currentframe())
+    self.assertEqual(set(['BP_ID']), self._completed)
+    self.assertLen(self._update_queue, 1)
+    self.assertGreater(len(self._update_queue[0]['stackFrames']), 3)
+    self.assertTrue(self._update_queue[0]['isFinalState'])
+
   def testDoubleHit(self):
     breakpoint = python_breakpoint.PythonBreakpoint(self._template, self, self,
                                                     None)
@@ -541,6 +557,18 @@ def testExpirationTime(self):
     self.assertEqual(
         datetime(year=2015, month=1, day=2), breakpoint.GetExpirationTime())
 
+  def testExpirationTimeUnixMsec(self):
+    # Using the Snapshot Debugger (Firebase backend) version of creation time
+    self._template.pop('createTime', None);
+    self._template[
+      'createTimeUnixMsec'] = python_test_util.DateTimeToUnixMsec(
+        self._base_time)
+    breakpoint = python_breakpoint.PythonBreakpoint(self._template, self, self,
+                                                    None)
+    breakpoint.Clear()
+    self.assertEqual(
+        self._base_time + timedelta(hours=24), breakpoint.GetExpirationTime())
+
   def testExpirationTimeWithExpiresIn(self):
     definition = self._template.copy()
     definition['expires_in'] = {
diff --git a/tests/py/python_test_util.py b/tests/py/python_test_util.py
index 26d1aef..ffac4da 100644
--- a/tests/py/python_test_util.py
+++ b/tests/py/python_test_util.py
@@ -106,6 +106,10 @@ def DateTimeToTimestampNew(t):
   """
   return t.strftime('%Y-%m-%dT%H:%M:%S') + 'Z'
 
+def DateTimeToUnixMsec(t):
+  """Returns the Unix time as in integer value in milliseconds"""
+  return int(t.timestamp() * 1000)
+
 
 def PackFrameVariable(breakpoint, name, frame=0, collection='locals'):
   """Finds local variable or argument by name.

From c4b54d448ec1ac2b6bf70ddb0dd3def379a17ff4 Mon Sep 17 00:00:00 2001
From: jasonborg <48138260+jasonborg@users.noreply.github.com>
Date: Tue, 21 Feb 2023 21:19:22 +0000
Subject: [PATCH 3/6] fix: Cleanup logs when Firebase RTDB does no exist (#76)

The emitted logs when the DB does not exist will now resemble:

```
I0221 20:25:54.661298 93984 firebase_client.py:402] Failed to check debuggee presence at cdbg/debuggees/d-93910f74/registrationTimeUnixMsec: NotFoundError('404 Not Found')
I0221 20:25:54.661362 93984 firebase_client.py:373] registering at https://my-project-cdbg.firebaseio.com, path: cdbg/debuggees/d-93910f74
I0221 20:25:54.690697 93984 firebase_client.py:390] Failed to register debuggee: NotFoundError('404 Not Found')
```
---
 src/googleclouddebugger/firebase_client.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/googleclouddebugger/firebase_client.py b/src/googleclouddebugger/firebase_client.py
index 474211a..01afccd 100644
--- a/src/googleclouddebugger/firebase_client.py
+++ b/src/googleclouddebugger/firebase_client.py
@@ -382,10 +382,10 @@ def _RegisterDebuggee(self):
 
       self.register_backoff.Succeeded()
       return (False, 0)  # Proceed immediately to subscribing to breakpoints.
-    except BaseException:
+    except BaseException as e:
       # There is no significant benefit to handing different exceptions
       # in different ways; we will log and retry regardless.
-      native.LogInfo(f'Failed to register debuggee: {traceback.format_exc()}')
+      native.LogInfo(f'Failed to register debuggee: {repr(e)}')
       return (True, self.register_backoff.Failed())
 
   def _CheckDebuggeePresence(self):
@@ -394,9 +394,8 @@ def _CheckDebuggeePresence(self):
       snapshot = firebase_admin.db.reference(path).get()
       # The value doesn't matter; just return true if there's any value.
       return snapshot is not None
-    except BaseException:
-      native.LogInfo(
-          f'Failed to check debuggee presence: {traceback.format_exc()}')
+    except BaseException as e:
+      native.LogInfo(f'Failed to check debuggee presence at {path}: {repr(e)}')
       return False
 
   def _MarkDebuggeeActive(self):

From e0c48db8aad24984c3772720f542f2720d985fb4 Mon Sep 17 00:00:00 2001
From: jasonborg <48138260+jasonborg@users.noreply.github.com>
Date: Tue, 28 Feb 2023 18:47:07 +0000
Subject: [PATCH 4/6] feat: Support default-rtdb instance. (#78)

Fixes #77
---
 src/googleclouddebugger/firebase_client.py |  99 +++++++---
 tests/py/firebase_client_test.py           | 208 +++++++++++++++------
 2 files changed, 232 insertions(+), 75 deletions(-)

diff --git a/src/googleclouddebugger/firebase_client.py b/src/googleclouddebugger/firebase_client.py
index 01afccd..8dbe30a 100644
--- a/src/googleclouddebugger/firebase_client.py
+++ b/src/googleclouddebugger/firebase_client.py
@@ -106,8 +106,10 @@ def __init__(self):
     self._mark_active_interval_sec = 60 * 60  # 1 hour in seconds
     self._new_updates = threading.Event()
     self._breakpoint_subscription = None
+    self._firebase_app = None
 
     # Events for unit testing.
+    self.connection_complete = threading.Event()
     self.registration_complete = threading.Event()
     self.subscription_complete = threading.Event()
 
@@ -116,6 +118,7 @@ def __init__(self):
     #
 
     # Delay before retrying failed request.
+    self.connect_backoff = backoff.Backoff()  # Connect to the DB.
     self.register_backoff = backoff.Backoff()  # Register debuggee.
     self.subscribe_backoff = backoff.Backoff()  # Subscribe to updates.
     self.update_backoff = backoff.Backoff()  # Update breakpoint.
@@ -193,7 +196,10 @@ def SetupAuth(self,
       service_account_json_file: JSON file to use for credentials. If not
           provided, will default to application default credentials.
       database_url: Firebase realtime database URL to be used.  If not
-          provided, will default to https://{project_id}-cdbg.firebaseio.com
+          provided, connect attempts to the following DBs will be made, in
+          order:
+            https://{project_id}-cdbg.firebaseio.com
+            https://{project_id}-default-rtdb.firebaseio.com
     Raises:
       NoProjectIdError: If the project id cannot be determined.
     """
@@ -220,11 +226,7 @@ def SetupAuth(self,
           'Please specify the project id using the --project_id flag.')
 
     self._project_id = project_id
-
-    if database_url:
-      self._database_url = database_url
-    else:
-      self._database_url = f'https://{self._project_id}-cdbg.firebaseio.com'
+    self._database_url = database_url
 
   def Start(self):
     """Starts the worker thread."""
@@ -287,15 +289,11 @@ def _MainThreadProc(self):
     which will run in its own thread.  That thread will be owned by
     self._breakpoint_subscription.
     """
-    # Note: if self._credentials is None, default app credentials will be used.
-    try:
-      firebase_admin.initialize_app(self._credentials,
-                                    {'databaseURL': self._database_url})
-    except ValueError:
-      native.LogWarning(
-          f'Failed to initialize firebase: {traceback.format_exc()}')
-      native.LogError('Failed to start debugger agent.  Giving up.')
-      return
+    connection_required, delay = True, 0
+    while connection_required:
+      time.sleep(delay)
+      connection_required, delay = self._ConnectToDb()
+    self.connection_complete.set()
 
     registration_required, delay = True, 0
     while registration_required:
@@ -343,6 +341,45 @@ def _StartMarkActiveTimer(self):
                                               self._MarkActiveTimerFunc)
     self._mark_active_timer.start()
 
+  def _ConnectToDb(self):
+    urls = [self._database_url] if self._database_url is not None else \
+      [f'https://{self._project_id}-cdbg.firebaseio.com',
+      f'https://{self._project_id}-default-rtdb.firebaseio.com']
+
+    for url in urls:
+      native.LogInfo(f'Attempting to connect to DB with url: {url}')
+
+      status, firebase_app = self._TryInitializeDbForUrl(url)
+      if status:
+        native.LogInfo(f'Successfully connected to DB with url: {url}')
+        self._database_url = url
+        self._firebase_app = firebase_app
+        self.connect_backoff.Succeeded()
+        return (False, 0)  # Proceed immediately to registering the debuggee.
+
+    return (True, self.connect_backoff.Failed())
+
+  def _TryInitializeDbForUrl(self, database_url):
+    # Note: if self._credentials is None, default app credentials will be used.
+    app = None
+    try:
+      app = firebase_admin.initialize_app(
+          self._credentials, {'databaseURL': database_url}, name='cdbg')
+
+      if self._CheckSchemaVersionPresence(app):
+        return True, app
+
+    except ValueError:
+      native.LogWarning(
+          f'Failed to initialize firebase: {traceback.format_exc()}')
+
+    # This is the failure path, if we hit here we must cleanup the app handle
+    if app is not None:
+      firebase_admin.delete_app(app)
+      app = None
+
+    return False, app
+
   def _RegisterDebuggee(self):
     """Single attempt to register the debuggee.
 
@@ -371,11 +408,12 @@ def _RegisterDebuggee(self):
       else:
         debuggee_path = f'cdbg/debuggees/{self._debuggee_id}'
         native.LogInfo(
-            f'registering at {self._database_url}, path: {debuggee_path}')
+            f'Registering at {self._database_url}, path: {debuggee_path}')
         debuggee_data = copy.deepcopy(debuggee)
         debuggee_data['registrationTimeUnixMsec'] = {'.sv': 'timestamp'}
         debuggee_data['lastUpdateTimeUnixMsec'] = {'.sv': 'timestamp'}
-        firebase_admin.db.reference(debuggee_path).set(debuggee_data)
+        firebase_admin.db.reference(debuggee_path,
+                                    self._firebase_app).set(debuggee_data)
 
       native.LogInfo(
           f'Debuggee registered successfully, ID: {self._debuggee_id}')
@@ -388,10 +426,21 @@ def _RegisterDebuggee(self):
       native.LogInfo(f'Failed to register debuggee: {repr(e)}')
       return (True, self.register_backoff.Failed())
 
+  def _CheckSchemaVersionPresence(self, firebase_app):
+    path = f'cdbg/schema_version'
+    try:
+      snapshot = firebase_admin.db.reference(path, firebase_app).get()
+      # The value doesn't matter; just return true if there's any value.
+      return snapshot is not None
+    except BaseException as e:
+      native.LogInfo(
+          f'Failed to check schema version presence at {path}: {repr(e)}')
+      return False
+
   def _CheckDebuggeePresence(self):
     path = f'cdbg/debuggees/{self._debuggee_id}/registrationTimeUnixMsec'
     try:
-      snapshot = firebase_admin.db.reference(path).get()
+      snapshot = firebase_admin.db.reference(path, self._firebase_app).get()
       # The value doesn't matter; just return true if there's any value.
       return snapshot is not None
     except BaseException as e:
@@ -402,7 +451,8 @@ def _MarkDebuggeeActive(self):
     active_path = f'cdbg/debuggees/{self._debuggee_id}/lastUpdateTimeUnixMsec'
     try:
       server_time = {'.sv': 'timestamp'}
-      firebase_admin.db.reference(active_path).set(server_time)
+      firebase_admin.db.reference(active_path,
+                                  self._firebase_app).set(server_time)
     except BaseException:
       native.LogInfo(
           f'Failed to mark debuggee active: {traceback.format_exc()}')
@@ -415,7 +465,7 @@ def _SubscribeToBreakpoints(self):
 
     path = f'cdbg/breakpoints/{self._debuggee_id}/active'
     native.LogInfo(f'Subscribing to breakpoint updates at {path}')
-    ref = firebase_admin.db.reference(path)
+    ref = firebase_admin.db.reference(path, self._firebase_app)
     try:
       self._breakpoint_subscription = ref.listen(self._ActiveBreakpointCallback)
       return (False, 0)
@@ -508,7 +558,8 @@ def _TransmitBreakpointUpdates(self):
 
         # First, remove from the active breakpoints.
         bp_ref = firebase_admin.db.reference(
-            f'cdbg/breakpoints/{self._debuggee_id}/active/{bp_id}')
+            f'cdbg/breakpoints/{self._debuggee_id}/active/{bp_id}',
+            self._firebase_app)
         bp_ref.delete()
 
         summary_data = breakpoint_data
@@ -516,7 +567,8 @@ def _TransmitBreakpointUpdates(self):
         if is_snapshot:
           # Note that there may not be snapshot data.
           bp_ref = firebase_admin.db.reference(
-              f'cdbg/breakpoints/{self._debuggee_id}/snapshot/{bp_id}')
+              f'cdbg/breakpoints/{self._debuggee_id}/snapshot/{bp_id}',
+              self._firebase_app)
           bp_ref.set(breakpoint_data)
 
           # Now strip potential snapshot data.
@@ -527,7 +579,8 @@ def _TransmitBreakpointUpdates(self):
 
         # Then add it to the list of final breakpoints.
         bp_ref = firebase_admin.db.reference(
-            f'cdbg/breakpoints/{self._debuggee_id}/final/{bp_id}')
+            f'cdbg/breakpoints/{self._debuggee_id}/final/{bp_id}',
+            self._firebase_app)
         bp_ref.set(summary_data)
 
         native.LogInfo(f'Breakpoint {bp_id} update transmitted successfully')
diff --git a/tests/py/firebase_client_test.py b/tests/py/firebase_client_test.py
index 5cd8fb6..d72c68c 100644
--- a/tests/py/firebase_client_test.py
+++ b/tests/py/firebase_client_test.py
@@ -6,6 +6,7 @@
 import tempfile
 import time
 from unittest import mock
+from unittest.mock import ANY
 from unittest.mock import MagicMock
 from unittest.mock import call
 from unittest.mock import patch
@@ -20,6 +21,7 @@
 
 import firebase_admin.credentials
 from firebase_admin.exceptions import FirebaseError
+from firebase_admin.exceptions import NotFoundError
 
 TEST_PROJECT_ID = 'test-project-id'
 METADATA_PROJECT_URL = ('http://metadata.google.internal/computeMetadata/'
@@ -61,8 +63,8 @@ def setUp(self):
 
     # Speed up the delays for retry loops.
     for backoff in [
-        self._client.register_backoff, self._client.subscribe_backoff,
-        self._client.update_backoff
+        self._client.connect_backoff, self._client.register_backoff,
+        self._client.subscribe_backoff, self._client.update_backoff
     ]:
       backoff.min_interval_sec /= 100000.0
       backoff.max_interval_sec /= 100000.0
@@ -73,19 +75,33 @@ def setUp(self):
     self._mock_initialize_app = patcher.start()
     self.addCleanup(patcher.stop)
 
+    patcher = patch('firebase_admin.delete_app')
+    self._mock_delete_app = patcher.start()
+    self.addCleanup(patcher.stop)
+
     patcher = patch('firebase_admin.db.reference')
     self._mock_db_ref = patcher.start()
     self.addCleanup(patcher.stop)
 
     # Set up the mocks for the database refs.
+    self._firebase_app = 'FIREBASE_APP_HANDLE'
+    self._mock_initialize_app.return_value = self._firebase_app
+    self._mock_schema_version_ref = MagicMock()
+    self._mock_schema_version_ref.get.return_value = "2"
     self._mock_presence_ref = MagicMock()
     self._mock_presence_ref.get.return_value = None
     self._mock_active_ref = MagicMock()
     self._mock_register_ref = MagicMock()
     self._fake_subscribe_ref = FakeReference()
+
+    # Setup common happy path reference sequence:
+    # cdbg/schema_version
+    # cdbg/debuggees/{debuggee_id}/registrationTimeUnixMsec
+    # cdbg/debuggees/{debuggee_id}
+    # cdbg/breakpoints/{debuggee_id}/active
     self._mock_db_ref.side_effect = [
-        self._mock_presence_ref, self._mock_register_ref,
-        self._fake_subscribe_ref
+        self._mock_schema_version_ref, self._mock_presence_ref,
+        self._mock_register_ref, self._fake_subscribe_ref
     ]
 
   def tearDown(self):
@@ -100,8 +116,6 @@ def testSetupAuthDefault(self):
       self._client.SetupAuth()
 
     self.assertEqual(TEST_PROJECT_ID, self._client._project_id)
-    self.assertEqual(f'https://{TEST_PROJECT_ID}-cdbg.firebaseio.com',
-                     self._client._database_url)
 
   def testSetupAuthOverrideProjectIdNumber(self):
     # If a project id is provided, we use it.
@@ -109,8 +123,6 @@ def testSetupAuthOverrideProjectIdNumber(self):
     self._client.SetupAuth(project_id=project_id)
 
     self.assertEqual(project_id, self._client._project_id)
-    self.assertEqual(f'https://{project_id}-cdbg.firebaseio.com',
-                     self._client._database_url)
 
   def testSetupAuthServiceAccountJsonAuth(self):
     # We'll load credentials from the provided file (mocked for simplicity)
@@ -142,11 +154,14 @@ def testStart(self):
     debuggee_id = self._client._debuggee_id
 
     self._mock_initialize_app.assert_called_with(
-        None, {'databaseURL': f'https://{TEST_PROJECT_ID}-cdbg.firebaseio.com'})
+        None, {'databaseURL': f'https://{TEST_PROJECT_ID}-cdbg.firebaseio.com'},
+        name='cdbg')
     self.assertEqual([
-        call(f'cdbg/debuggees/{debuggee_id}/registrationTimeUnixMsec'),
-        call(f'cdbg/debuggees/{debuggee_id}'),
-        call(f'cdbg/breakpoints/{debuggee_id}/active')
+        call(f'cdbg/schema_version', self._firebase_app),
+        call(f'cdbg/debuggees/{debuggee_id}/registrationTimeUnixMsec',
+             self._firebase_app),
+        call(f'cdbg/debuggees/{debuggee_id}', self._firebase_app),
+        call(f'cdbg/breakpoints/{debuggee_id}/active', self._firebase_app)
     ], self._mock_db_ref.call_args_list)
 
     # Verify that the register call has been made.
@@ -155,13 +170,97 @@ def testStart(self):
     expected_data['lastUpdateTimeUnixMsec'] = {'.sv': 'timestamp'}
     self._mock_register_ref.set.assert_called_once_with(expected_data)
 
+  def testStartCustomDbUrlConfigured(self):
+    self._client.SetupAuth(
+        project_id=TEST_PROJECT_ID,
+        database_url='https://custom-db.firebaseio.com')
+    self._client.Start()
+    self._client.connection_complete.wait()
+
+    debuggee_id = self._client._debuggee_id
+
+    self._mock_initialize_app.assert_called_once_with(
+        None, {'databaseURL': 'https://custom-db.firebaseio.com'}, name='cdbg')
+
+  def testStartConnectFallsBackToDefaultRtdb(self):
+    # A new schema_version ref will be fetched each time
+    self._mock_db_ref.side_effect = [
+        self._mock_schema_version_ref, self._mock_schema_version_ref,
+        self._mock_presence_ref, self._mock_register_ref,
+        self._fake_subscribe_ref
+    ]
+
+    # Fail on the '-cdbg' instance test, succeed on the '-default-rtdb' one.
+    self._mock_schema_version_ref.get.side_effect = [
+        NotFoundError("Not found", http_response=404), '2'
+    ]
+
+    self._client.SetupAuth(project_id=TEST_PROJECT_ID)
+    self._client.Start()
+    self._client.connection_complete.wait()
+
+    self.assertEqual([
+        call(
+            None,
+            {'databaseURL': f'https://{TEST_PROJECT_ID}-cdbg.firebaseio.com'},
+            name='cdbg'),
+        call(
+            None, {
+                'databaseURL':
+                    f'https://{TEST_PROJECT_ID}-default-rtdb.firebaseio.com'
+            },
+            name='cdbg')
+    ], self._mock_initialize_app.call_args_list)
+
+    self.assertEqual(1, self._mock_delete_app.call_count)
+
+  def testStartConnectFailsThenSucceeds(self):
+    # A new schema_version ref will be fetched each time
+    self._mock_db_ref.side_effect = [
+        self._mock_schema_version_ref, self._mock_schema_version_ref,
+        self._mock_schema_version_ref, self._mock_presence_ref,
+        self._mock_register_ref, self._fake_subscribe_ref
+    ]
+
+    # Completely fail on the initial attempt at reaching a DB, then succeed on
+    # 2nd attempt. One full attempt will try the '-cdbg' db instance followed by
+    # the '-default-rtdb' one.
+    self._mock_schema_version_ref.get.side_effect = [
+        NotFoundError("Not found", http_response=404),
+        NotFoundError("Not found", http_response=404), '2'
+    ]
+
+    self._client.SetupAuth(project_id=TEST_PROJECT_ID)
+    self._client.Start()
+    self._client.connection_complete.wait()
+
+    self.assertEqual([
+        call(
+            None,
+            {'databaseURL': f'https://{TEST_PROJECT_ID}-cdbg.firebaseio.com'},
+            name='cdbg'),
+        call(
+            None, {
+                'databaseURL':
+                    f'https://{TEST_PROJECT_ID}-default-rtdb.firebaseio.com'
+            },
+            name='cdbg'),
+        call(
+            None,
+            {'databaseURL': f'https://{TEST_PROJECT_ID}-cdbg.firebaseio.com'},
+            name='cdbg')
+    ], self._mock_initialize_app.call_args_list)
+
+    self.assertEqual(2, self._mock_delete_app.call_count)
+
   def testStartAlreadyPresent(self):
     # Create a mock for just this test that claims the debuggee is registered.
     mock_presence_ref = MagicMock()
     mock_presence_ref.get.return_value = 'present!'
 
     self._mock_db_ref.side_effect = [
-        mock_presence_ref, self._mock_active_ref, self._fake_subscribe_ref
+        self._mock_schema_version_ref, mock_presence_ref, self._mock_active_ref,
+        self._fake_subscribe_ref
     ]
 
     self._client.SetupAuth(project_id=TEST_PROJECT_ID)
@@ -171,9 +270,12 @@ def testStartAlreadyPresent(self):
     debuggee_id = self._client._debuggee_id
 
     self.assertEqual([
-        call(f'cdbg/debuggees/{debuggee_id}/registrationTimeUnixMsec'),
-        call(f'cdbg/debuggees/{debuggee_id}/lastUpdateTimeUnixMsec'),
-        call(f'cdbg/breakpoints/{debuggee_id}/active')
+        call(f'cdbg/schema_version', self._firebase_app),
+        call(f'cdbg/debuggees/{debuggee_id}/registrationTimeUnixMsec',
+             self._firebase_app),
+        call(f'cdbg/debuggees/{debuggee_id}/lastUpdateTimeUnixMsec',
+             self._firebase_app),
+        call(f'cdbg/breakpoints/{debuggee_id}/active', self._firebase_app)
     ], self._mock_db_ref.call_args_list)
 
     # Verify that the register call has been made.
@@ -182,9 +284,9 @@ def testStartAlreadyPresent(self):
   def testStartRegisterRetry(self):
     # A new set of db refs are fetched on each retry.
     self._mock_db_ref.side_effect = [
-        self._mock_presence_ref, self._mock_register_ref,
-        self._mock_presence_ref, self._mock_register_ref,
-        self._fake_subscribe_ref
+        self._mock_schema_version_ref, self._mock_presence_ref,
+        self._mock_register_ref, self._mock_presence_ref,
+        self._mock_register_ref, self._fake_subscribe_ref
     ]
 
     # Fail once, then succeed on retry.
@@ -202,6 +304,7 @@ def testStartSubscribeRetry(self):
 
     # A new db ref is fetched on each retry.
     self._mock_db_ref.side_effect = [
+        self._mock_schema_version_ref,
         self._mock_presence_ref,
         self._mock_register_ref,
         mock_subscribe_ref,  # Fail the first time
@@ -212,28 +315,27 @@ def testStartSubscribeRetry(self):
     self._client.Start()
     self._client.subscription_complete.wait()
 
-    self.assertEqual(4, self._mock_db_ref.call_count)
+    self.assertEqual(5, self._mock_db_ref.call_count)
 
   def testMarkActiveTimer(self):
-      # Make sure that there are enough refs queued up.
-      refs = list(self._mock_db_ref.side_effect)
-      refs.extend([self._mock_active_ref] * 10)
-      self._mock_db_ref.side_effect = refs
-
-      # Speed things WAY up rather than waiting for hours.
-      self._client._mark_active_interval_sec = 0.1
+    # Make sure that there are enough refs queued up.
+    refs = list(self._mock_db_ref.side_effect)
+    refs.extend([self._mock_active_ref] * 10)
+    self._mock_db_ref.side_effect = refs
 
-      self._client.SetupAuth(project_id=TEST_PROJECT_ID)
-      self._client.Start()
-      self._client.subscription_complete.wait()
+    # Speed things WAY up rather than waiting for hours.
+    self._client._mark_active_interval_sec = 0.1
 
-      # wait long enough for the timer to trigger a few times.
-      time.sleep(0.5)
+    self._client.SetupAuth(project_id=TEST_PROJECT_ID)
+    self._client.Start()
+    self._client.subscription_complete.wait()
 
-      print(f'Timer triggered {self._mock_active_ref.set.call_count} times')
-      self.assertTrue(self._mock_active_ref.set.call_count > 3)
-      self._mock_active_ref.set.assert_called_with({'.sv': 'timestamp'})
+    # wait long enough for the timer to trigger a few times.
+    time.sleep(0.5)
 
+    print(f'Timer triggered {self._mock_active_ref.set.call_count} times')
+    self.assertTrue(self._mock_active_ref.set.call_count > 3)
+    self._mock_active_ref.set.assert_called_with({'.sv': 'timestamp'})
 
   def testBreakpointSubscription(self):
     # This class will keep track of the breakpoint updates and will check
@@ -310,9 +412,9 @@ def testEnqueueBreakpointUpdate(self):
     final_ref_mock = MagicMock()
 
     self._mock_db_ref.side_effect = [
-        self._mock_presence_ref, self._mock_register_ref,
-        self._fake_subscribe_ref, active_ref_mock, snapshot_ref_mock,
-        final_ref_mock
+        self._mock_schema_version_ref, self._mock_presence_ref,
+        self._mock_register_ref, self._fake_subscribe_ref, active_ref_mock,
+        snapshot_ref_mock, final_ref_mock
     ]
 
     self._client.SetupAuth(project_id=TEST_PROJECT_ID)
@@ -369,14 +471,14 @@ def testEnqueueBreakpointUpdate(self):
 
     db_ref_calls = self._mock_db_ref.call_args_list
     self.assertEqual(
-        call(f'cdbg/breakpoints/{debuggee_id}/active/{breakpoint_id}'),
-        db_ref_calls[3])
+        call(f'cdbg/breakpoints/{debuggee_id}/active/{breakpoint_id}',
+             self._firebase_app), db_ref_calls[4])
     self.assertEqual(
-        call(f'cdbg/breakpoints/{debuggee_id}/snapshot/{breakpoint_id}'),
-        db_ref_calls[4])
+        call(f'cdbg/breakpoints/{debuggee_id}/snapshot/{breakpoint_id}',
+             self._firebase_app), db_ref_calls[5])
     self.assertEqual(
-        call(f'cdbg/breakpoints/{debuggee_id}/final/{breakpoint_id}'),
-        db_ref_calls[5])
+        call(f'cdbg/breakpoints/{debuggee_id}/final/{breakpoint_id}',
+             self._firebase_app), db_ref_calls[6])
 
     active_ref_mock.delete.assert_called_once()
     snapshot_ref_mock.set.assert_called_once_with(full_breakpoint)
@@ -387,8 +489,9 @@ def testEnqueueBreakpointUpdateWithLogpoint(self):
     final_ref_mock = MagicMock()
 
     self._mock_db_ref.side_effect = [
-        self._mock_presence_ref, self._mock_register_ref,
-        self._fake_subscribe_ref, active_ref_mock, final_ref_mock
+        self._mock_schema_version_ref, self._mock_presence_ref,
+        self._mock_register_ref, self._fake_subscribe_ref, active_ref_mock,
+        final_ref_mock
     ]
 
     self._client.SetupAuth(project_id=TEST_PROJECT_ID)
@@ -436,19 +539,19 @@ def testEnqueueBreakpointUpdateWithLogpoint(self):
 
     db_ref_calls = self._mock_db_ref.call_args_list
     self.assertEqual(
-        call(f'cdbg/breakpoints/{debuggee_id}/active/{breakpoint_id}'),
-        db_ref_calls[3])
+        call(f'cdbg/breakpoints/{debuggee_id}/active/{breakpoint_id}',
+             self._firebase_app), db_ref_calls[4])
     self.assertEqual(
-        call(f'cdbg/breakpoints/{debuggee_id}/final/{breakpoint_id}'),
-        db_ref_calls[4])
+        call(f'cdbg/breakpoints/{debuggee_id}/final/{breakpoint_id}',
+             self._firebase_app), db_ref_calls[5])
 
     active_ref_mock.delete.assert_called_once()
     final_ref_mock.set.assert_called_once_with(output_breakpoint)
 
     # Make sure that the snapshot node was not accessed.
     self.assertTrue(
-        call(f'cdbg/breakpoints/{debuggee_id}/snapshot/{breakpoint_id}') not in
-        db_ref_calls)
+        call(f'cdbg/breakpoints/{debuggee_id}/snapshot/{breakpoint_id}', ANY)
+        not in db_ref_calls)
 
   def testEnqueueBreakpointUpdateRetry(self):
     active_ref_mock = MagicMock()
@@ -468,6 +571,7 @@ def testEnqueueBreakpointUpdateRetry(self):
     ]
 
     self._mock_db_ref.side_effect = [
+        self._mock_schema_version_ref,
         self._mock_presence_ref,
         self._mock_register_ref,
         self._fake_subscribe_ref,  # setup

From 42f5fc31a1d4c1d8a91eaa4d6c51a0773dae7d93 Mon Sep 17 00:00:00 2001
From: jasonborg <48138260+jasonborg@users.noreply.github.com>
Date: Wed, 1 Mar 2023 18:26:52 +0000
Subject: [PATCH 5/6] fix: Module not found corner case (#80)

Fixes #79
---
 src/googleclouddebugger/module_utils.py      | 25 ++++++++++++++++++++
 src/googleclouddebugger/python_breakpoint.py | 15 +-----------
 tests/py/module_utils_test.py                |  7 ++++++
 3 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/googleclouddebugger/module_utils.py b/src/googleclouddebugger/module_utils.py
index 738fc8c..53f2e37 100644
--- a/src/googleclouddebugger/module_utils.py
+++ b/src/googleclouddebugger/module_utils.py
@@ -16,6 +16,26 @@
 import os
 import sys
 
+def NormalizePath(path):
+  """Normalizes a path.
+
+  E.g. One example is it will convert "/a/b/./c" -> "/a/b/c"
+  """
+  # TODO: Calling os.path.normpath "may change the meaning of a
+  # path that contains symbolic links" (e.g., "A/foo/../B" != "A/B" if foo is a
+  # symlink). This might cause trouble when matching against loaded module
+  # paths. We should try to avoid using it.
+  # Example:
+  #  > import symlink.a
+  #  > symlink.a.__file__
+  #  symlink/a.py
+  #  > import target.a
+  #  > starget.a.__file__
+  #  target/a.py
+  # Python interpreter treats these as two separate modules. So, we also need to
+  # handle them the same way.
+  return os.path.normpath(path)
+
 
 def IsPathSuffix(mod_path, path):
   """Checks whether path is a full path suffix of mod_path.
@@ -69,6 +89,11 @@ def GetLoadedModuleBySuffix(path):
     if not os.path.isabs(mod_root):
       mod_root = os.path.join(os.getcwd(), mod_root)
 
+    # In the following invocation 'python3 ./main.py' (using the ./), the
+    # mod_root variable will '/base/path/./main'. In order to correctly compare
+    # it with the root variable, it needs to be '/base/path/main'.
+    mod_root = NormalizePath(mod_root)
+
     if IsPathSuffix(mod_root, root):
       return module
 
diff --git a/src/googleclouddebugger/python_breakpoint.py b/src/googleclouddebugger/python_breakpoint.py
index c65b95d..62f2512 100644
--- a/src/googleclouddebugger/python_breakpoint.py
+++ b/src/googleclouddebugger/python_breakpoint.py
@@ -134,20 +134,7 @@ def _MultipleModulesFoundError(path, candidates):
 
 def _NormalizePath(path):
   """Removes surrounding whitespace, leading separator and normalize."""
-  # TODO: Calling os.path.normpath "may change the meaning of a
-  # path that contains symbolic links" (e.g., "A/foo/../B" != "A/B" if foo is a
-  # symlink). This might cause trouble when matching against loaded module
-  # paths. We should try to avoid using it.
-  # Example:
-  #  > import symlink.a
-  #  > symlink.a.__file__
-  #  symlink/a.py
-  #  > import target.a
-  #  > starget.a.__file__
-  #  target/a.py
-  # Python interpreter treats these as two separate modules. So, we also need to
-  # handle them the same way.
-  return os.path.normpath(path.strip().lstrip(os.sep))
+  return module_utils.NormalizePath(path.strip().lstrip(os.sep))
 
 
 class PythonBreakpoint(object):
diff --git a/tests/py/module_utils_test.py b/tests/py/module_utils_test.py
index 0ed0fd2..ac847ad 100644
--- a/tests/py/module_utils_test.py
+++ b/tests/py/module_utils_test.py
@@ -156,6 +156,13 @@ def testMainLoadedModuleFromSuffix(self):
     self.assertTrue(m1, 'Module not found')
     self.assertEqual('/a/b/p/m.pyc', m1.__file__)
 
+  def testMainWithDotSlashLoadedModuleFromSuffix(self):
+    # Lookup module started via 'python3 ./m.py', notice the './'
+    _AddSysModule('__main__', '/a/b/p/./m.pyc')
+    m1 = module_utils.GetLoadedModuleBySuffix('/a/b/p/m.py')
+    self.assertIsNotNone(m1)
+    self.assertTrue(m1, 'Module not found')
+    self.assertEqual('/a/b/p/./m.pyc', m1.__file__)
 
 if __name__ == '__main__':
   absltest.main()

From ec24144b191c63bdf88f72d3b930823157035ca1 Mon Sep 17 00:00:00 2001
From: jasonborg <48138260+jasonborg@users.noreply.github.com>
Date: Wed, 1 Mar 2023 18:53:39 +0000
Subject: [PATCH 6/6] chore: Release version 3.4 (#81)

---
 src/googleclouddebugger/version.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/googleclouddebugger/version.py b/src/googleclouddebugger/version.py
index a61798c..5c21bc2 100644
--- a/src/googleclouddebugger/version.py
+++ b/src/googleclouddebugger/version.py
@@ -4,4 +4,4 @@
 # The major version should only change on breaking changes. Minor version
 # changes go between regular updates. Instances running debuggers with
 # different major versions will show up as two different debuggees.
-__version__ = '3.3'
+__version__ = '3.4'