mono / sdb

A command line client for the Mono soft debugger.
https://www.mono-project.com
MIT License
116 stars 44 forks source link

Handle debugger stop event with SoftDebuggerSession.TargetEvent #60

Open akihikodaki opened 4 years ago

akihikodaki commented 4 years ago

The debugger does not listen to some events fired by SoftDebuggerSession, and may miss a stop event, which resumes command line processing, and lead to hangup.

SoftDebuggerSession.TargetEvent handles all events and eliminates possibility to introduce a hangup also in the future.

alexrp commented 4 years ago

Just out of curiosity, which exact event(s) were we missing?

akihikodaki commented 4 years ago

TargetSignaled is a stop event but was not listened by sdb.

alexrp commented 4 years ago

Hmm. It seems to me that we would want to let the user know if such an event occurred, similar to how we say Inferior process suspended, Inferior process interrupted, etc. So I would suggest attaching an event to TargetSignaled (assuming that's exposed in the API) like so:

                _session.TargetSignaled += (sender, e) =>
                {
                    // maybe log the signal name/id as well, if available through the API on `e`?
                    Log.Notice("Inferior process '{0}' ('{1}') signaled",
                               ActiveProcess.Id, StringizeTarget());
                };
akihikodaki commented 4 years ago

Hmm. It seems to me that we would want to let the user know if such an event occurred, similar to how we say Inferior process suspended, Inferior process interrupted, etc. So I would suggest attaching an event to TargetSignaled (assuming that's exposed in the API) like so:

                _session.TargetSignaled += (sender, e) =>
                {
                    // maybe log the signal name/id as well, if available through the API on `e`?
                    Log.Notice("Inferior process '{0}' ('{1}') signaled",
                               ActiveProcess.Id, StringizeTarget());
                };

I think that would be nice. I suppose it requires another pull request.