-
Notifications
You must be signed in to change notification settings - Fork 68
Turning suspend all policy and using get stack trace command #731
Conversation
Codecov Report
@@ Coverage Diff @@
## master #731 +/- ##
==========================================
- Coverage 60.39% 60.04% -0.36%
==========================================
Files 27 27
Lines 3838 3834 -4
==========================================
- Hits 2318 2302 -16
- Misses 1520 1532 +12
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add single pydev command to suspend all threads and resume all threads
# TODO: Replace this with resume all command after #732 is fixed | ||
for pyd_tid in self.thread_map.pydevd_ids(): | ||
self.pydevd_notify(pydevd_comm.CMD_THREAD_RUN, pyd_tid) | ||
self.send_response(request, allThreadsContinued=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we have a single command that would tell pydevd to continue ALL threads. Feels a lot more efficient, and code world work better.
Also should be done for suspend all threads also If not already done. I.e. when we got the issue button we should send a single request to pause ALL threads instead of sending a request for each these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance pause will fail if a new thread is created in the interim (a race condition resulting in a bug). So technically it should be pydeb that does the necessary plumbing and iteration of threads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# this thread are now invalid; clear their IDs. | ||
with self.stack_traces_lock: | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌👌👌👌👌👌
This change does the following:
This should address most of the concerns with #72.