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.

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

Fix debug Linux #679

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

yuxiaomao
Copy link
Collaborator

I made some changes for the hashlink-debugger vscode extension on Linux vshaxe/hashlink-debugger@1.4.8...1.4.9 . They are not needed for the command line version (because you can't give command when the program is still in "running" mode).
As I also made a big hack for waitpid timeout (for the moment it works on my machine without need thread+semaphore), ping @ncannasse

@yuxiaomao
Copy link
Collaborator Author

My questions:

  • Maybe kill in hl_debug_stop should be at Haxe side (in this case there is a need to either always stop before detaching, or detect Linux - I'm not sure which choice is better)?
  • Is that ok to have a strange hack on waipid ? or maybe it's better to use a thread like for Mac ? or have a different behavior when running node and when running hashlink?

@@ -84,6 +85,7 @@ HL_API bool hl_debug_stop( int pid ) {
# elif defined(MAC_DEBUG)
return mdbg_session_detach(pid);
# elif defined(USE_PTRACE)
kill(pid, SIGTRAP); // DETACH needs ptrace-stop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SIGSTOP would be more appropriate here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. But when I test with SIGSTOP, the disconnect does not works properly (maybe SIGSTOP is not actually stopping - the signal is forward to our thread? or has some delay / different behavior than SIGTRAP). :(

Comment on lines +253 to +258
// *** HACK ***
// usleep here is needed for a good result.
// Without it, waitpid can miss many stop event;
// With it, and more we wait, less we miss stop event.
usleep(100 * 1000);
int ret = waitpid(pid, &status, WNOHANG);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that waitpid with WNOHANG is also vulnerable to a race condition with libuv.

I'm not sure how to fix this other than by doing the same thread & semaphore dance as the Mac debugger.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also suspect the waitpid hack was for libuv (or vscode) waiting for all childs.

I tested your implementation with thread (it helps me a lot!), but it failed when we are actually waiting (by default the vscode extension call wait with a timeout = 0) and has a timeout. I didn't managed to get it works yet :'(
You said @ncannasse refused your PR due to complexity, so I tried this hack. I will do the thread & semaphore if hack is not a good option either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was another PR, I never actually submitted the thread & semaphore implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I misunderstood :(

@yuxiaomao yuxiaomao marked this pull request as draft September 6, 2024 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants