Properly validate breakpoints. Fixes #1191 (#1462)

src/ptvsd/_vendored/pydevd/_pydev_bundle/pydev_log.py

@@ -99,6 +99,11 @@ def error_once(msg, *args):
         critical(message)
 
 
+def debug_once(msg, *args):
+    if DebugInfoHolder.DEBUG_TRACE_LEVEL >= 3:
+        error_once(msg, *args)
+
+
 def show_compile_cython_command_line():
     if SHOW_COMPILE_CYTHON_COMMAND_LINE:
         dirname = os.path.dirname(os.path.dirname(__file__))

src/ptvsd/_vendored/pydevd/_pydevd_bundle/pydevd_api.py

@@ -250,6 +250,27 @@ def filename_to_server(self, filename):
         filename = self.filename_to_str(filename)
         return pydevd_file_utils.norm_file_to_server(filename)
 
+    class _DummyFrame(object):
+        '''
+        Dummy frame to be used with PyDB.apply_files_filter (as we don't really have the
+        related frame as breakpoints are added before execution).
+        '''
+
+        class _DummyCode(object):
+
+            def __init__(self, filename):
+                self.co_firstlineno = 1
+                self.co_filename = filename
+                self.co_name = 'invalid func name '
+
+        def __init__(self, filename):
+            self.f_code = self._DummyCode(filename)
+            self.f_globals = {}
+
+    ADD_BREAKPOINT_NO_ERROR = 0
+    ADD_BREAKPOINT_FILE_NOT_FOUND = 1
+    ADD_BREAKPOINT_FILE_EXCLUDED_BY_FILTERS = 2
+
     def add_breakpoint(
             self, py_db, filename, breakpoint_type, breakpoint_id, line, condition, func_name, expression, suspend_policy, hit_condition, is_logpoint):
         '''
@@ -285,14 +306,34 @@ def add_breakpoint(
         :param bool is_logpoint:
             If True and an expression is passed, pydevd will create an io message command with the
             result of the evaluation.
+
+        :return int:
+            :see: ADD_BREAKPOINT_NO_ERROR = 0
+            :see: ADD_BREAKPOINT_FILE_NOT_FOUND = 1
+            :see: ADD_BREAKPOINT_FILE_EXCLUDED_BY_FILTERS = 2
         '''
         assert filename.__class__ == str  # i.e.: bytes on py2 and str on py3
         assert func_name.__class__ == str  # i.e.: bytes on py2 and str on py3
 
         if not pydevd_file_utils.exists(filename):
-            pydev_log.critical('pydev debugger: warning: trying to add breakpoint'\
-                ' to file that does not exist: %s (will have no effect)\n' % (filename,))
-            return
+            return self.ADD_BREAKPOINT_FILE_NOT_FOUND
+
+        error_code = self.ADD_BREAKPOINT_NO_ERROR
+        if (
+                py_db.is_files_filter_enabled and
+                not py_db.get_require_module_for_filters() and
+                py_db.apply_files_filter(self._DummyFrame(filename), filename, False)
+            ):
+            # Note that if `get_require_module_for_filters()` returns False, we don't do this check.
+            # This is because we don't have the module name given a file at this point (in
+            # runtime it's gotten from the frame.f_globals).
+            # An option could be calculate it based on the filename and current sys.path,
+            # but on some occasions that may be wrong (for instance with `__main__` or if
+            # the user dynamically changes the PYTHONPATH).
+
+            # Note: depending on the use-case, filters may be changed, so, keep on going and add the
+            # breakpoint even with the error code.
+            error_code = self.ADD_BREAKPOINT_FILE_EXCLUDED_BY_FILTERS
 
         if breakpoint_type == 'python-line':
             added_breakpoint = LineBreakpoint(line, condition, func_name, expression, suspend_policy, hit_condition=hit_condition, is_logpoint=is_logpoint)
@@ -329,6 +370,7 @@ def add_breakpoint(
             py_db.has_plugin_line_breaks = py_db.plugin.has_line_breaks()
 
         py_db.on_breakpoints_changed()
+        return error_code
 
     def remove_all_breakpoints(self, py_db, filename):
         '''

src/ptvsd/_vendored/pydevd/_pydevd_bundle/pydevd_comm.py

@@ -227,7 +227,10 @@ def _read(self, size):
                 self._buffer = self._buffer[size:]
                 return ret
 
-            r = self.sock.recv(max(size - buffer_len, 1024))
+            try:
+                r = self.sock.recv(max(size - buffer_len, 1024))
+            except OSError:
+                return b''
             if not r:
                 return b''
             self._buffer += r
@@ -241,7 +244,10 @@ def _read_line(self):
                 self._buffer = self._buffer[i:]
                 return ret
             else:
-                r = self.sock.recv(1024)
+                try:
+                    r = self.sock.recv(1024)
+                except OSError:
+                    return b''
                 if not r:
                     return b''
                 self._buffer += r

src/ptvsd/_vendored/pydevd/_pydevd_bundle/pydevd_filtering.py

@@ -127,22 +127,23 @@ def __init__(self):
 
         # Stepping filters.
         pydevd_filters = os.getenv('PYDEVD_FILTERS', '')
-        if pydevd_filters.startswith('{'):
-            # dict(glob_pattern (str) -> exclude(True or False))
-            exclude_filters = []
-            for key, val in json.loads(pydevd_filters).items():
-                exclude_filters.append(ExcludeFilter(key, val, True))
-            self._exclude_filters = exclude_filters
-        else:
-            # A ';' separated list of strings with globs for the
-            # list of excludes.
-            filters = pydevd_filters.split(';')
-            pydev_log.debug("PYDEVD_FILTERS %s\n" % filters)
-            new_filters = []
-            for new_filter in filters:
-                if new_filter.strip():
-                    new_filters.append(ExcludeFilter(new_filter.strip(), True, True))
-            self._exclude_filters = new_filters
+        if pydevd_filters:
+            pydev_log.debug("PYDEVD_FILTERS %s", (pydevd_filters,))
+            if pydevd_filters.startswith('{'):
+                # dict(glob_pattern (str) -> exclude(True or False))
+                exclude_filters = []
+                for key, val in json.loads(pydevd_filters).items():
+                    exclude_filters.append(ExcludeFilter(key, val, True))
+                self._exclude_filters = exclude_filters
+            else:
+                # A ';' separated list of strings with globs for the
+                # list of excludes.
+                filters = pydevd_filters.split(';')
+                new_filters = []
+                for new_filter in filters:
+                    if new_filter.strip():
+                        new_filters.append(ExcludeFilter(new_filter.strip(), True, True))
+                self._exclude_filters = new_filters
 
     @classmethod
     def _get_default_library_roots(cls):
@@ -276,8 +277,6 @@ def exclude_by_filter(self, filename, module_name):
         for exclude_filter in self._exclude_filters:  # : :type exclude_filter: ExcludeFilter
             if exclude_filter.is_path:
                 if glob_matches_path(filename, exclude_filter.name):
-                    if exclude_filter.exclude:
-                        pydev_log.debug("File %s ignored by filter %s" % (filename, exclude_filter.name))
                     return exclude_filter.exclude
             else:
                 # Module filter.

src/ptvsd/_vendored/pydevd/_pydevd_bundle/pydevd_process_net_command.py

@@ -257,9 +257,20 @@ def cmd_set_break(self, py_db, cmd_id, seq, text):
         filename = self.api.filename_to_server(filename)
         func_name = self.api.to_str(func_name)
 
-        self.api.add_breakpoint(
+        error_code = self.api.add_breakpoint(
             py_db, filename, btype, breakpoint_id, line, condition, func_name, expression, suspend_policy, hit_condition, is_logpoint)
 
+        if error_code:
+            if error_code == self.api.ADD_BREAKPOINT_FILE_NOT_FOUND:
+                pydev_log.critical('pydev debugger: warning: Trying to add breakpoint to file that does not exist: %s (will have no effect).' % (filename,))
+
+            elif error_code == self.api.ADD_BREAKPOINT_FILE_EXCLUDED_BY_FILTERS:
+                pydev_log.critical('pydev debugger: warning: Trying to add breakpoint to file that is excluded by filters: %s (will have no effect).' % (filename,))
+
+            else:
+                # Shouldn't get here.
+                pydev_log.critical('pydev debugger: warning: Breakpoint not validated (reason unknown -- please report as error): %s.' % (filename,))
+
     def cmd_remove_break(self, py_db, cmd_id, seq, text):
         # command to remove some breakpoint
         # text is type\file\tid. Remove from breakpoints dictionary

src/ptvsd/_vendored/pydevd/_pydevd_bundle/pydevd_process_net_command_json.py

@@ -9,20 +9,20 @@
 
 import pydevd_file_utils
 from _pydev_bundle import pydev_log
-from _pydevd_bundle._debug_adapter import pydevd_base_schema
+from _pydevd_bundle._debug_adapter import pydevd_base_schema, pydevd_schema
 from _pydevd_bundle._debug_adapter.pydevd_schema import (
     CompletionsResponseBody, EvaluateResponseBody, ExceptionOptions,
     GotoTargetsResponseBody, ModulesResponseBody, ProcessEventBody,
 	ProcessEvent, Scope, ScopesResponseBody, SetExpressionResponseBody,
 	SetVariableResponseBody, SourceBreakpoint, SourceResponseBody,
-	VariablesResponseBody)
+	VariablesResponseBody, SetBreakpointsResponseBody)
 from _pydevd_bundle.pydevd_api import PyDevdAPI
 from _pydevd_bundle.pydevd_breakpoints import get_exception_class
 from _pydevd_bundle.pydevd_comm_constants import (
     CMD_PROCESS_EVENT, CMD_RETURN, CMD_SET_NEXT_STATEMENT, CMD_STEP_INTO,
 	CMD_STEP_INTO_MY_CODE, CMD_STEP_OVER, CMD_STEP_OVER_MY_CODE,
 	CMD_STEP_RETURN, CMD_STEP_RETURN_MY_CODE)
-from _pydevd_bundle.pydevd_constants import DebugInfoHolder, IS_WINDOWS
+from _pydevd_bundle.pydevd_constants import DebugInfoHolder
 from _pydevd_bundle.pydevd_filtering import ExcludeFilter
 from _pydevd_bundle.pydevd_json_debug_options import _extract_debug_options
 from _pydevd_bundle.pydevd_net_command import NetCommand
@@ -115,8 +115,8 @@ def _convert_rules_to_exclude_filters(rules, filename_to_server, on_error):
         # i.e.: if we have:
         # /sub1/sub2/sub3
         # a rule with /sub1/sub2 would match before a rule only with /sub1.
-        directory_exclude_filters = sorted(directory_exclude_filters, key=lambda exclude_filter: -len(exclude_filter.name))
-        module_exclude_filters = sorted(module_exclude_filters, key=lambda exclude_filter: -len(exclude_filter.name))
+        directory_exclude_filters = sorted(directory_exclude_filters, key=lambda exclude_filter:-len(exclude_filter.name))
+        module_exclude_filters = sorted(module_exclude_filters, key=lambda exclude_filter:-len(exclude_filter.name))
         exclude_filters = directory_exclude_filters + glob_exclude_filters + module_exclude_filters
 
     return exclude_filters
@@ -193,14 +193,10 @@ def on_request(py_db, request):
             if DEBUG:
                 print('Handled in pydevd: %s (in _PyDevJsonCommandProcessor).\n' % (method_name,))
 
-        py_db._main_lock.acquire()
-        try:
-
+        with py_db._main_lock:
             cmd = on_request(py_db, request)
             if cmd is not None:
                 py_db.writer.add_command(cmd)
-        finally:
-            py_db._main_lock.release()
 
     def on_configurationdone_request(self, py_db, request):
         '''
@@ -462,6 +458,19 @@ def on_setbreakpoints_request(self, py_db, request):
         '''
         :param SetBreakpointsRequest request:
         '''
+        if not self._launch_or_attach_request_done:
+            # Note that to validate the breakpoints we need the launch request to be done already
+            # (otherwise the filters wouldn't be set for the breakpoint validation).
+            body = SetBreakpointsResponseBody([])
+            response = pydevd_base_schema.build_response(
+                request,
+                kwargs={
+                    'body': body,
+                    'success': False,
+                    'message': 'Breakpoints may only be set after the launch request is received.'
+                })
+            return NetCommand(CMD_RETURN, 0, response, is_json=True)
+
         arguments = request.arguments  # : :type arguments: SetBreakpointsArguments
         # TODO: Path is optional here it could be source reference.
         filename = arguments.source.path
@@ -510,13 +519,30 @@ def on_setbreakpoints_request(self, py_db, request):
                 is_logpoint = True
                 expression = convert_dap_log_message_to_expression(log_message)
 
-            self.api.add_breakpoint(
+            error_code = self.api.add_breakpoint(
                 py_db, filename, btype, breakpoint_id, line, condition, func_name, expression, suspend_policy, hit_condition, is_logpoint)
 
-            # Note that the id is made up (the id for pydevd is unique only within a file, so, the
-            # line is used for it).
-            # Also, the id is currently not used afterwards, so, we don't even keep a mapping.
-            breakpoints_set.append({'id': self._next_breakpoint_id(), 'verified': True, 'line': line})
+            if error_code:
+                if error_code == self.api.ADD_BREAKPOINT_FILE_NOT_FOUND:
+                    error_msg = 'Breakpoint in file that does not exist.'
+
+                elif error_code == self.api.ADD_BREAKPOINT_FILE_EXCLUDED_BY_FILTERS:
+                    error_msg = 'Breakpoint in file excluded by filters.'
+                    if py_db.get_use_libraries_filter():
+                        error_msg += '\nNote: may be excluded because of "justMyCode" option (default == true).'
+
+                else:
+                    # Shouldn't get here.
+                    error_msg = 'Breakpoint not validated (reason unknown -- please report as bug).'
+
+                breakpoints_set.append(pydevd_schema.Breakpoint(
+                    verified=False, line=line, message=error_msg).to_dict())
+            else:
+                # Note that the id is made up (the id for pydevd is unique only within a file, so, the
+                # line is used for it).
+                # Also, the id is currently not used afterwards, so, we don't even keep a mapping.
+                breakpoints_set.append(pydevd_schema.Breakpoint(
+                    verified=True, id=self._next_breakpoint_id(), line=line).to_dict())
 
         body = {'breakpoints': breakpoints_set}
         set_breakpoints_response = pydevd_base_schema.build_response(request, kwargs={'body': body})

src/ptvsd/_vendored/pydevd/pydevd.py

@@ -202,12 +202,12 @@ def _on_run(self):
                     pydev_log.debug("No threads alive, finishing debug session")
                     self.py_db.finish_debugging_session()
                     kill_all_pydev_threads()
+                    self.wait_pydb_threads_to_finish()
                 except:
                     pydev_log.exception()
 
                 self.killReceived = True
-
-            self.wait_pydb_threads_to_finish()
+                return
 
             self.py_db.check_output_redirect()
 
@@ -218,7 +218,7 @@ def wait_pydb_threads_to_finish(self, timeout=0.5):
         while time.time() < started_at + timeout:
             if len(pydb_daemon_threads) == 1 and pydb_daemon_threads.get(self, None):
                 return
-            time.sleep(0.01)
+            time.sleep(1 / 30.)
         pydev_log.debug("The following pydb threads may not have finished correctly: %s",
                         ', '.join([t.getName() for t in pydb_daemon_threads if t is not self]))
 
@@ -775,22 +775,23 @@ def _exclude_by_filter(self, frame, filename):
         :return: True if it should be excluded, False if it should be included and None
             if no rule matched the given file.
         '''
+        cache_key = (filename, frame.f_code.co_name)
         try:
-            return self._exclude_by_filter_cache[filename]
+            return self._exclude_by_filter_cache[cache_key]
         except KeyError:
             cache = self._exclude_by_filter_cache
 
             abs_real_path_and_basename = get_abs_path_real_path_and_base_from_file(filename)
             # pydevd files are always filtered out
             if self.get_file_type(abs_real_path_and_basename) == self.PYDEV_FILE:
-                cache[filename] = True
+                cache[cache_key] = True
             else:
                 module_name = None
                 if self._files_filtering.require_module:
-                    module_name = frame.f_globals.get('__name__')
-                cache[filename] = self._files_filtering.exclude_by_filter(filename, module_name)
+                    module_name = frame.f_globals.get('__name__', '')
+                cache[cache_key] = self._files_filtering.exclude_by_filter(filename, module_name)
 
-            return cache[filename]
+            return cache[cache_key]
 
     def apply_files_filter(self, frame, filename, force_check_project_scope):
         '''
@@ -814,7 +815,7 @@ def apply_files_filter(self, frame, filename, force_check_project_scope):
             if self.plugin is not None and (self.has_plugin_line_breaks or self.has_plugin_exception_breaks):
                 # If it's explicitly needed by some plugin, we can't skip it.
                 if not self.plugin.can_skip(self, frame):
-                    # print('include (include by plugins): %s' % filename)
+                    pydev_log.debug_once('File traced (included by plugins): %s', filename)
                     self._apply_filter_cache[cache_key] = False
                     return False
 
@@ -823,21 +824,30 @@ def apply_files_filter(self, frame, filename, force_check_project_scope):
                 if exclude_by_filter is not None:
                     if exclude_by_filter:
                         # ignore files matching stepping filters
-                        # print('exclude (filtered out): %s' % filename)
+                        pydev_log.debug_once('File not traced (excluded by filters): %s', filename)
+
                         self._apply_filter_cache[cache_key] = True
                         return True
                     else:
-                        # print('include (explicitly included): %s' % filename)
+                        pydev_log.debug_once('File traced (explicitly included by filters): %s', filename)
+
                         self._apply_filter_cache[cache_key] = False
                         return False
 
             if (self._is_libraries_filter_enabled or force_check_project_scope) and not self.in_project_scope(filename):
-                # print('exclude (not on project): %s' % filename)
                 # ignore library files while stepping
                 self._apply_filter_cache[cache_key] = True
+                if force_check_project_scope:
+                    pydev_log.debug_once('File not traced (not in project): %s', filename)
+                else:
+                    pydev_log.debug_once('File not traced (not in project - force_check_project_scope): %s', filename)
+
                 return True
 
-            # print('include (on project): %s' % filename)
+            if force_check_project_scope:
+                pydev_log.debug_once('File traced: %s (force_check_project_scope)', filename)
+            else:
+                pydev_log.debug_once('File traced: %s', filename)
             self._apply_filter_cache[cache_key] = False
             return False
 
@@ -880,6 +890,9 @@ def set_use_libraries_filter(self, use_libraries_filter):
     def get_use_libraries_filter(self):
         return self._files_filtering.use_libraries_filter()
 
+    def get_require_module_for_filters(self):
+        return self._files_filtering.require_module
+
     def has_threads_alive(self):
         for t in pydevd_utils.get_non_pydevd_threads():
             if isinstance(t, PyDBDaemonThread):