Skip to content

Commit

Permalink
Don't use pydevd_find_thread_by_id unless really needed. Fixes #1129
Browse files Browse the repository at this point in the history
  • Loading branch information
fabioz committed Dec 8, 2022
1 parent 79b7c56 commit bf5c644
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 38 deletions.
3 changes: 2 additions & 1 deletion src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,13 @@ def request_stack(self, py_db, seq, thread_id, fmt=None, timeout=.5, start_frame
else:
py_db.post_internal_command(internal_get_thread_stack, '*')

def request_exception_info_json(self, py_db, request, thread_id, max_frames):
def request_exception_info_json(self, py_db, request, thread_id, thread, max_frames):
py_db.post_method_as_internal_command(
thread_id,
internal_get_exception_details_json,
request,
thread_id,
thread,
max_frames,
set_additional_thread_info=set_additional_thread_info,
iter_visible_frames_info=py_db.cmd_factory._iter_visible_frames_info,
Expand Down
13 changes: 6 additions & 7 deletions src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_comm.py
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ def do_it(self, dbg):

def internal_step_in_thread(py_db, thread_id, cmd_id, set_additional_thread_info):
thread_to_step = pydevd_find_thread_by_id(thread_id)
if thread_to_step:
if thread_to_step is not None:
info = set_additional_thread_info(thread_to_step)
info.pydev_original_step_cmd = cmd_id
info.pydev_step_cmd = cmd_id
Expand All @@ -682,7 +682,7 @@ def internal_step_in_thread(py_db, thread_id, cmd_id, set_additional_thread_info

def internal_smart_step_into(py_db, thread_id, offset, child_offset, set_additional_thread_info):
thread_to_step = pydevd_find_thread_by_id(thread_id)
if thread_to_step:
if thread_to_step is not None:
info = set_additional_thread_info(thread_to_step)
info.pydev_original_step_cmd = CMD_SMART_STEP_INTO
info.pydev_step_cmd = CMD_SMART_STEP_INTO
Expand Down Expand Up @@ -714,7 +714,7 @@ def __init__(self, thread_id, cmd_id, line, func_name, seq=0):

def do_it(self, dbg):
t = pydevd_find_thread_by_id(self.thread_id)
if t:
if t is not None:
info = t.additional_info
info.pydev_original_step_cmd = self.cmd_id
info.pydev_step_cmd = self.cmd_id
Expand Down Expand Up @@ -1401,11 +1401,10 @@ def internal_get_description(dbg, seq, thread_id, frame_id, expression):
dbg.writer.add_command(cmd)


def build_exception_info_response(dbg, thread_id, request_seq, set_additional_thread_info, iter_visible_frames_info, max_frames):
def build_exception_info_response(dbg, thread_id, thread, request_seq, set_additional_thread_info, iter_visible_frames_info, max_frames):
'''
:return ExceptionInfoResponse
'''
thread = pydevd_find_thread_by_id(thread_id)
additional_info = set_additional_thread_info(thread)
topmost_frame = additional_info.get_topmost_frame(thread)

Expand Down Expand Up @@ -1540,11 +1539,11 @@ def build_exception_info_response(dbg, thread_id, request_seq, set_additional_th
return response


def internal_get_exception_details_json(dbg, request, thread_id, max_frames, set_additional_thread_info=None, iter_visible_frames_info=None):
def internal_get_exception_details_json(dbg, request, thread_id, thread, max_frames, set_additional_thread_info=None, iter_visible_frames_info=None):
''' Fetch exception details
'''
try:
response = build_exception_info_response(dbg, thread_id, request.seq, set_additional_thread_info, iter_visible_frames_info, max_frames)
response = build_exception_info_response(dbg, thread_id, thread, request.seq, set_additional_thread_info, iter_visible_frames_info, max_frames)
except:
exc = get_exception_traceback_str()
response = pydevd_base_schema.build_response(request, kwargs={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
from _pydevd_bundle.pydevd_additional_thread_info import set_additional_thread_info
from _pydevd_bundle import pydevd_frame_utils, pydevd_constants, pydevd_utils
import linecache
from _pydevd_bundle.pydevd_thread_lifecycle import pydevd_find_thread_by_id
from io import StringIO
from _pydev_bundle import pydev_log

Expand Down Expand Up @@ -346,10 +345,9 @@ def make_console_message(self, msg):
])

@overrides(NetCommandFactory.make_thread_suspend_single_notification)
def make_thread_suspend_single_notification(self, py_db, thread_id, stop_reason):
def make_thread_suspend_single_notification(self, py_db, thread_id, thread, stop_reason):
exc_desc = None
exc_name = None
thread = pydevd_find_thread_by_id(thread_id)
info = set_additional_thread_info(thread)

preserve_focus_hint = False
Expand All @@ -375,7 +373,7 @@ def make_thread_suspend_single_notification(self, py_db, thread_id, stop_reason)

if stop_reason == 'exception':
exception_info_response = build_exception_info_response(
py_db, thread_id, -1, set_additional_thread_info, self._iter_visible_frames_info, max_frames=-1)
py_db, thread_id, thread, -1, set_additional_thread_info, self._iter_visible_frames_info, max_frames=-1)
exception_info_response

exc_name = exception_info_response.body.exceptionId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def make_thread_suspend_message(self, py_db, thread_id, frames_list, stop_reason
except:
return self.make_error_message(0, get_exception_traceback_str())

def make_thread_suspend_single_notification(self, py_db, thread_id, stop_reason):
def make_thread_suspend_single_notification(self, py_db, thread_id, thread, stop_reason):
try:
return NetCommand(CMD_THREAD_SUSPEND_SINGLE_NOTIFICATION, 0, json.dumps(
{'thread_id': thread_id, 'stop_reason':stop_reason}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -743,10 +743,11 @@ def cmd_get_exception_details(self, py_db, cmd_id, seq, text):
thread_id = text
t = pydevd_find_thread_by_id(thread_id)
frame = None
if t and not getattr(t, 'pydev_do_not_trace', None):
if t is not None and not getattr(t, 'pydev_do_not_trace', None):
additional_info = set_additional_thread_info(t)
frame = additional_info.get_topmost_frame(t)
try:
# Note: provide the return even if the thread is empty.
return py_db.cmd_factory.make_get_exception_details_message(py_db, seq, thread_id, frame)
finally:
frame = None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,16 @@ def on_stepin_request(self, py_db, request):
target_id = arguments.targetId
if target_id is not None:
thread = pydevd_find_thread_by_id(thread_id)
if thread is None:
response = Response(
request_seq=request.seq,
success=False,
command=request.command,
message='Unable to find thread from thread_id: %s' % (thread_id,),
body={},
)
return NetCommand(CMD_RETURN, 0, response, is_json=True)

info = set_additional_thread_info(thread)
target_id_to_smart_step_into_variant = info.target_id_to_smart_step_into_variant
if not target_id_to_smart_step_into_variant:
Expand Down Expand Up @@ -978,7 +988,18 @@ def on_exceptioninfo_request(self, py_db, request):
exception_into_arguments = request.arguments
thread_id = exception_into_arguments.threadId
max_frames = self._options.max_exception_stack_frames
self.api.request_exception_info_json(py_db, request, thread_id, max_frames)
thread = pydevd_find_thread_by_id(thread_id)
if thread is not None:
self.api.request_exception_info_json(py_db, request, thread_id, thread, max_frames)
else:
response = Response(
request_seq=request.seq,
success=False,
command=request.command,
message='Unable to find thread from thread_id: %s' % (thread_id,),
body={},
)
return NetCommand(CMD_RETURN, 0, response, is_json=True)

def on_scopes_request(self, py_db, request):
'''
Expand Down
4 changes: 2 additions & 2 deletions src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ def on_timeout_unblock_threads():
on_timeout_unblock_threads.called = True
pydev_log.info('Resuming threads after evaluate timeout.')
resume_threads('*', except_thread=curr_thread)
py_db.threads_suspended_single_notification.on_thread_resume(tid)
py_db.threads_suspended_single_notification.on_thread_resume(tid, curr_thread)

on_timeout_unblock_threads.called = False

Expand All @@ -341,7 +341,7 @@ def on_timeout_unblock_threads():
mark_thread_suspended(curr_thread, CMD_SET_BREAK)
py_db.threads_suspended_single_notification.increment_suspend_time()
suspend_all_threads(py_db, except_thread=curr_thread)
py_db.threads_suspended_single_notification.on_thread_suspend(tid, CMD_SET_BREAK)
py_db.threads_suspended_single_notification.on_thread_suspend(tid, curr_thread, CMD_SET_BREAK)


def _evaluate_with_timeouts(original_func):
Expand Down
41 changes: 22 additions & 19 deletions src/debugpy/_vendored/pydevd/pydevd.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ class AbstractSingleNotificationBehavior(object):
# On do_wait_suspend, use notify_thread_suspended:
def do_wait_suspend(...):
with single_notification_behavior.notify_thread_suspended(thread_id):
with single_notification_behavior.notify_thread_suspended(thread_id, thread, reason):
...
'''

Expand All @@ -308,7 +308,7 @@ def do_wait_suspend(...):
'_lock',
'_next_request_time',
'_suspend_time_request',
'_suspended_thread_ids',
'_suspended_thread_id_to_thread',
'_pause_requested',
'_py_db',
]
Expand All @@ -322,10 +322,10 @@ def __init__(self, py_db):
self._last_resume_notification_time = -1
self._suspend_time_request = self._next_request_time()
self._lock = thread.allocate_lock()
self._suspended_thread_ids = set()
self._suspended_thread_id_to_thread = {}
self._pause_requested = False

def send_suspend_notification(self, thread_id, stop_reason):
def send_suspend_notification(self, thread_id, thread, stop_reason):
raise AssertionError('abstract: subclasses must override.')

def send_resume_notification(self, thread_id):
Expand All @@ -351,29 +351,30 @@ def on_pause(self):

def _notify_after_timeout(self, global_suspend_time):
with self._lock:
if self._suspended_thread_ids:
if self._suspended_thread_id_to_thread:
if global_suspend_time > self._last_suspend_notification_time:
self._last_suspend_notification_time = global_suspend_time
# Notify about any thread which is currently suspended.
pydev_log.info('Sending suspend notification after timeout.')
self.send_suspend_notification(next(iter(self._suspended_thread_ids)), CMD_THREAD_SUSPEND)
thread_id, thread = next(iter(self._suspended_thread_id_to_thread.items()))
self.send_suspend_notification(thread_id, thread, CMD_THREAD_SUSPEND)

def on_thread_suspend(self, thread_id, stop_reason):
def on_thread_suspend(self, thread_id, thread, stop_reason):
with self._lock:
pause_requested = self._pause_requested
if pause_requested:
# When a suspend notification is sent, reset the pause flag.
self._pause_requested = False

self._suspended_thread_ids.add(thread_id)
self._suspended_thread_id_to_thread[thread_id] = thread

# CMD_THREAD_SUSPEND should always be a side-effect of a break, so, only
# issue for a CMD_THREAD_SUSPEND if a pause is pending.
if stop_reason != CMD_THREAD_SUSPEND or pause_requested:
if self._suspend_time_request > self._last_suspend_notification_time:
pydev_log.info('Sending suspend notification.')
self._last_suspend_notification_time = self._suspend_time_request
self.send_suspend_notification(thread_id, stop_reason)
self.send_suspend_notification(thread_id, thread, stop_reason)
else:
pydev_log.info(
'Suspend not sent (it was already sent). Last suspend % <= Last resume %s',
Expand All @@ -385,10 +386,10 @@ def on_thread_suspend(self, thread_id, stop_reason):
'Suspend not sent because stop reason is thread suspend and pause was not requested.',
)

def on_thread_resume(self, thread_id):
def on_thread_resume(self, thread_id, thread):
# on resume (step, continue all):
with self._lock:
self._suspended_thread_ids.remove(thread_id)
self._suspended_thread_id_to_thread.pop(thread_id)
if self._last_resume_notification_time < self._last_suspend_notification_time:
pydev_log.info('Sending resume notification.')
self._last_resume_notification_time = self._last_suspend_notification_time
Expand All @@ -401,12 +402,12 @@ def on_thread_resume(self, thread_id):
)

@contextmanager
def notify_thread_suspended(self, thread_id, stop_reason):
self.on_thread_suspend(thread_id, stop_reason)
def notify_thread_suspended(self, thread_id, thread, stop_reason):
self.on_thread_suspend(thread_id, thread, stop_reason)
try:
yield # At this point the thread must be actually suspended.
finally:
self.on_thread_resume(thread_id)
self.on_thread_resume(thread_id, thread)


class ThreadsSuspendedSingleNotification(AbstractSingleNotificationBehavior):
Expand Down Expand Up @@ -439,16 +440,18 @@ def send_resume_notification(self, thread_id):
callback()

@overrides(AbstractSingleNotificationBehavior.send_suspend_notification)
def send_suspend_notification(self, thread_id, stop_reason):
def send_suspend_notification(self, thread_id, thread, stop_reason):
py_db = self._py_db()
if py_db is not None:
py_db.writer.add_command(py_db.cmd_factory.make_thread_suspend_single_notification(py_db, thread_id, stop_reason))
py_db.writer.add_command(
py_db.cmd_factory.make_thread_suspend_single_notification(
py_db, thread_id, thread, stop_reason))

@overrides(AbstractSingleNotificationBehavior.notify_thread_suspended)
@contextmanager
def notify_thread_suspended(self, thread_id, stop_reason):
def notify_thread_suspended(self, thread_id, thread, stop_reason):
if self.multi_threads_single_notification:
with AbstractSingleNotificationBehavior.notify_thread_suspended(self, thread_id, stop_reason):
with AbstractSingleNotificationBehavior.notify_thread_suspended(self, thread_id, thread, stop_reason):
yield
else:
yield
Expand Down Expand Up @@ -2063,7 +2066,7 @@ def do_wait_suspend(self, thread, frame, event, arg, exception_type=None): # @U

from_this_thread.append(frame_custom_thread_id)

with self._threads_suspended_single_notification.notify_thread_suspended(thread_id, stop_reason):
with self._threads_suspended_single_notification.notify_thread_suspended(thread_id, thread, stop_reason):
keep_suspended = self._do_wait_suspend(thread, frame, event, arg, suspend_type, from_this_thread, frames_tracker)

frames_list = None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from _pydevd_bundle.pydevd_daemon_thread import run_as_pydevd_daemon_thread
from tests_python.debugger_unittest import CMD_THREAD_SUSPEND, CMD_STEP_OVER, CMD_SET_BREAK
from _pydev_bundle.pydev_override import overrides
import threading

STATE_RUN = 1
STATE_SUSPEND = 2
Expand All @@ -19,6 +20,7 @@ class _ThreadInfo(object):

def __init__(self):
self.state = STATE_RUN
self.thread = threading.Thread()
self.thread_id = self.next_thread_id()


Expand Down Expand Up @@ -47,7 +49,7 @@ def send_suspend_notification(self, *args, **kwargs):
self.notification_queue.put('suspend')

def do_wait_suspend(self, thread_info, stop_reason):
with self.notify_thread_suspended(thread_info.thread_id, stop_reason=stop_reason):
with self.notify_thread_suspended(thread_info.thread_id, thread_info.thread, stop_reason=stop_reason):
while thread_info.state == STATE_SUSPEND:
time.sleep(.1)

Expand Down Expand Up @@ -93,7 +95,9 @@ def join_thread(t):
class _DummyPyDB(object):

def __init__(self):
from _pydevd_bundle.pydevd_timeout import TimeoutTracker
self.created_pydb_daemon_threads = {}
self.timeout_tracker = TimeoutTracker(self)


def test_single_notification_1(single_notification_behavior, notification_queue):
Expand Down Expand Up @@ -230,7 +234,7 @@ def test_single_notification_3(single_notification_behavior, notification_queue,

join_thread(t2)
assert notification_queue.qsize() == 0
assert not single_notification_behavior._suspended_thread_ids
assert not single_notification_behavior._suspended_thread_id_to_thread

# Now, no threads are running and pause is pressed
# (maybe we could do a thread dump in this case as this
Expand Down

0 comments on commit bf5c644

Please sign in to comment.