[debugger enhance] don't block gdbserver thread while executing #989
Conversation
| * in the cluster, otherwise ignore this event */ | ||
| status = 0; | ||
|
|
||
| /* By design, all the other threads should stopped at this |
There was a problem hiding this comment.
should stop or should have been stopped?
| /* Resume all threads so they can receive the TERM signal */ | ||
| exec_env->current_status->running_status = STATUS_RUNNING; | ||
| os_cond_signal(&exec_env->wait_cond); |
There was a problem hiding this comment.
Should we wrap os_cond_signal with os_mutex_lock(&exec_env->wait_lock) and os_mutex_unlock(&exec_env->wait_lock)
| request = inbuf[0]; | ||
| payload = (char *)&inbuf[1]; |
There was a problem hiding this comment.
Will the inbuf's length be no smaller than 2? Shall we check the length here?
There was a problem hiding this comment.
Add a check for request. If it's '\0', that means this is an empty request, and we ignore it
| payload = (char *)&inbuf[1]; | ||
|
|
||
| LOG_VERBOSE("receive request:%c %s\n", request, payload); | ||
| handler_packet(server, request, payload); |
There was a problem hiding this comment.
How about changing handle_packet?
| { | ||
| if (ctx->receive_index >= sizeof(ctx->receive_buffer)) { | ||
| LOG_ERROR("RSP message buffer overflow"); | ||
| bh_assert(false); |
| tid = wasm_debug_instance_wait_thread( | ||
| (WASMDebugInstance *)server->thread->debug_instance, tid, &status); | ||
| /* Waiting for the stop event */ | ||
| while (!debug_inst->stopped_thread) { |
There was a problem hiding this comment.
Dead loop occupies a lot CPU resource, is it possible to wait a signal? Or add a timeout wait inside the loop?
| /* lock the debug_inst->wait_lock so | ||
| other threads can't fire stop events */ | ||
| os_mutex_lock(&cluster->debug_inst->wait_lock); | ||
| while (cluster->debug_inst->stopped_thread == exec_env) { |
There was a problem hiding this comment.
Same as above, can we add os_cond_timedwait(&cluster->debug_inst->wait_cond, ..) inside loop?
There was a problem hiding this comment.
Done, thanks!
| if (request == '\0') { | ||
| LOG_VERBOSE("ignore empty request"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Should move these lines before L149? If inbuf[0] is '\0', we should not read inbuf[1] in L149?
…codealliance#989) Allow to set break point again when all break points are removed and wasm app starts running.
2 participants
No description provided.