Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3.2.9] Concurrent debuggers can hang Jadeite #927

Closed
rjsargent opened this issue Aug 12, 2022 · 5 comments
Closed

[3.2.9] Concurrent debuggers can hang Jadeite #927

rjsargent opened this issue Aug 12, 2022 · 5 comments
Assignees
@ericwinger
Labels
BlackDiamond Bugs that are effortful to fix bug Something isn't working FixInFirstCut P2-Significant
Projects
Support Rowan 3.0

Comments

@rjsargent
Copy link
Member

Highlight an expression in the source pane, choose "Debug" from the pop up menu, nothing happens.
Evaluate something like "self halt printString" in the inspector text pane of the debugger and you can open a second debugger. However, closing the second debugger freezes Jadeite. UI is completely unresponsive with a spinner cursor.

It is possible that the problem is caused by there being an error in the object's printString.
@rjsargent
Copy link
Member Author

SIGUSR1 stack report attached in gem log.
gemnetobject27597galbadia.log

@LisaAlmarode
Copy link
Member

The second debugger scenario is quite bad. If you execute self halt printString in the middle right pane of the debugger, various things may happen:

  • hard freeze
  • no immediate freeze, the selecting contexts in the debugger puts up an error dialog.
  • no immediate freeze, but going to the transcript in order to logout causes a freeze.

@LisaAlmarode LisaAlmarode changed the title Concurrent debuggers is problematic [3.2.9] Concurrent debuggers can hang Jadeite Aug 22, 2022
@LisaAlmarode LisaAlmarode added the bug Something isn't working label Aug 22, 2022
@LisaAlmarode LisaAlmarode added this to Triage in Support Rowan 3.0 via automation Aug 22, 2022
@LisaAlmarode LisaAlmarode moved this from Triage to For Lisa Review in Support Rowan 3.0 Aug 25, 2022
@LisaAlmarode LisaAlmarode moved this from For Lisa Review to to-do in Support Rowan 3.0 Aug 30, 2022
@ericwinger ericwinger added the BlackDiamond Bugs that are effortful to fix label Aug 31, 2022
ericwinger pushed a commit that referenced this issue Oct 3, 2022
First cut - attempted fix #927 hanging debuggers
7d7c130 
#927

Instead of forking our own processes off to wait for an answer when a debugger opens on a remote process, we now send `deferredValue` to a block which executes asynchronously but returns synchronously.

So far, it's showing promise because the debugger tests are passing and multiple debuggers aren't hanging the system in ad-hoc testing. Need more rigorous testing though.
ericwinger pushed a commit that referenced this issue Oct 4, 2022
add debugger tests + test fixes
6fc74ef 
#927

Added two very simple debugger tests which open multiple debuggers.

Ensure jadeiteShell is updated in the TestCase instance after the test resource logs in

accelerator changes necessitate test changes

Better (I hope) closing of debuggers in tests. Certainly faster as delay decreased in tearDown code

Replaced some nil tests in the tests with isKindOf: because debugger tests do nasty things to windows to get rid of them.

Don't fail assertion of debugger semaphore in setUp method unless debuggers present

transcript test needed tab to be initialized lazily before test.
@ericwinger
Copy link
Member

I've reworked the debugger code in question to be much simpler and hopefully more robust. See checkins for details but, in short, we're using a deferredValue to do the asynchronous waiting instead of the poorly constructed forking code before. See JadeiteDebugger>>answer for the main changes. Closing for Lisa review.

Support Rowan 3.0 automation moved this from to-do to For Lisa Review Oct 4, 2022
@LisaAlmarode
Copy link
Member

The scenario in this bug results in a hard freeze in 3.2.11. Easy to reproduce:

  • In transcript workspace, do a debug-it on something e.g. Array with: 1.
  • In the resulting debugger, execute-it on self halt printString.
  • Close second debugger using upper right window x.
  • See freeze... open taskmanager to kill jadeite.

@LisaAlmarode LisaAlmarode reopened this Oct 6, 2022
Support Rowan 3.0 automation moved this from For Lisa Review to to-do Oct 6, 2022
ericwinger pushed a commit that referenced this issue Oct 11, 2022
More improvements to debugger so it doesn't freeze
eafda47 
#927

Note - most changes in this commit are category changes which were done automatically when the package was saved rather than saving packages.

Main work was in JadePresenter class>>waitForAnswer: No longer use a semaphore because the loopWhile: block doesn't necessarily run the ensure: block after termination. Also handle errors more effectively.

Added test which gets an error in the debugged code requiring the stepThrough calls to be forked or else the test won't finish.
ericwinger pushed a commit that referenced this issue Oct 11, 2022
if debugger is closed, make sure to terminate the forked loopWhile:
141ee90 
#927

Added simple test based on Lisa's reproduction case. test_debuggerDoesNotFreeze2
ericwinger pushed a commit that referenced this issue Oct 12, 2022
Don't kill processes
0eae33c 
#927

Sending `kill` to a Dolphin process won't run termination blocks. I've seen cases where the Dolphin UI event handler seems to get stuck after killing a process looping over the events. Sending `terminate` instead which seems to alleviate the problem.
ericwinger pushed a commit that referenced this issue Oct 13, 2022
debugger answer method now loops safely / fixed process selection bug
067f899 
#927

No need to send `loopWhile:` in JadeiteDebugger>>answer.

When selecting processes, make sure the process list in the gui is checked before making the selection. And use an `at:ifAbsent:`

Also, while waiting for an answer, if the loop is terminated, double check that the deferred value is available and return that if so.
ericwinger pushed a commit that referenced this issue Oct 14, 2022
work in progress - debugger hung again on simple change to source
82bf4d9 
#927

Did find, what I think, is a better way to wait for a debugger answer. Using ModalMsgLoop which blocks the main thread without blocking the messaging queue so the gui won't freeze.
ericwinger pushed a commit that referenced this issue Oct 18, 2022
work in progress - debugger wait code revamped again
744f163 
#927
The ModalMsgLoop will not work. Blocks the main process. This wait code is more promising. It suspends the
amswering process and forks off a process to wait on the deferred value. Make sure to resume the answering process as a main process or the resume will fail. Terminate answering process if the debugger has been closed.

So far, tests (both running and debugging) & ad-hoc testing look ok.
ericwinger pushed a commit that referenced this issue Oct 19, 2022
fixed several new debugger problems
1d9c304 
#927

Debugger tests would hang because the idle semaphore was signaled, but not from a main thread. Apparently, you can't signal a semaphore in a forked process from a non-main thread. Not helpful.

Debugger tests were asserting browsers were closed which isn't going to work.

Restored Browse Class debugger source context menu item. Lost somewhere along the way.
@ericwinger
Copy link
Member

Lots of work on the debugger freezing Jadeite has been done including a complete rewrite of the debugger's waiting code which was the source of the problem. (Dolphin green threads are tricky) Lisa hasn't been able to reproduce it in 3.2.12 prereleases. I'm going to close this as fixed and we should create a new bug with a new reproduction case if it rears its ugly head again.

Support Rowan 3.0 automation moved this from to-do to For Lisa Review Nov 1, 2022
@LisaAlmarode LisaAlmarode moved this from For Lisa Review to Done in Support Rowan 3.0 Nov 2, 2022
@ericwinger ericwinger mentioned this issue Nov 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ericwinger ericwinger

Labels
BlackDiamond Bugs that are effortful to fix bug Something isn't working FixInFirstCut P2-Significant
Projects
Support Rowan 3.0
  
Done
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rjsargent @LisaAlmarode @ericwinger