13
\$\begingroup\$

I have an application that needs to always be responsive. I've written a class that is designed to monitor the UI thread. The goal is to provide useful information in the logs to be able to understand when the UI thread becomes unresponsive and to determine which code is the cause.

To do this I check how long it takes to process something on the UI thread, if its longer than a certain threshold, I log a warning. If this happens multiple times in succession, it prints the stacktrace.

I've had this in production for about 2 weeks now, and so far it is working as expected. However, given that it uses deprecated methods to get the stacktrace, I'd like to know if this might end up causing more problems than it solves.

 public class ThreadMonitor
 {
    private static readonly ILog Log =  LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
    private readonly Thread thread;
    private readonly int pollingFrequency;
    private readonly int delayThreshold;
    private readonly int stackTraceIterations;

    public ThreadMonitor(Thread thread, int pollingFrequency = 500, int delayThreshold = 100, int stackTraceIterations = 4)
    {
        this.thread = thread;
        this.pollingFrequency = pollingFrequency;
        this.delayThreshold = delayThreshold;
        this.stackTraceIterations = stackTraceIterations;
    }

    public void Run()
    {
        Task.Factory.StartNew(() =>
        {
            while (true)
            {
                Thread.Sleep(pollingFrequency);
                var dispatcher = Dispatcher.FromThread(thread);
                var task = dispatcher.InvokeAsync(() => { });

                for (var i = 0; i < stackTraceIterations; i++)
                {
                    Thread.Sleep(delayThreshold);
                    if (task.Status != DispatcherOperationStatus.Completed)
                    {
                        Log.Debug($"{(i + 1) * 100}ms Delay on thread {thread.Name} ({task.Status})");
                    }
                    else
                    {
                        break;
                    }
                }

                if (task.Status == DispatcherOperationStatus.Completed)
                    continue;

                var stackTrace = GetStackTrace(thread);
                Log.Debug($"StackTrace of UI Thread: {stackTrace}");
            }
        });
    }

    #pragma warning disable 0618
    private StackTrace GetStackTrace(Thread targetThread)
    {
        StackTrace stackTrace = null;
        var ready = new ManualResetEventSlim();

        new Thread(() =>
        {
            // Backstop to release thread in case of deadlock:
            ready.Set();
            Thread.Sleep(200);
            try { targetThread.Resume(); } catch { }
        }).Start();

        ready.Wait();
        targetThread.Suspend();
        try { stackTrace = new StackTrace(targetThread, true); }
        catch { /* Deadlock */ }
        finally
        {
            try { targetThread.Resume(); }
            catch { stackTrace = null;  /* Deadlock */  }
        }

        return stackTrace;
    }
    #pragma warning restore 0618
 }
\$\endgroup\$
  • 1
    \$\begingroup\$ Typo in parameter name in ThreadMonitor and ThreadMonitor field name -- pollingFrequencey, I think you mean pollingFrequency. \$\endgroup\$ – jrh Jul 13 '17 at 21:06
3
\$\begingroup\$

Great idea! That makes it possible to get information about hanging / lacking GUIs. That kind of (probably) valuable information are only available via customer feedback otherwise :). It would be interesting to know if you already use that ThreadMonitor, if you already evaluated the log files and if you got some interesting information out of it ;).

I have just a few small comments to the code:

  • There is no need to use a Task for the infinity loop. You can just use a Thread here.
  • However, when using a Task, I would start it with TaskCreationOptions.LongRunning. Otherwise, a thread from the thread pool will be used (and blocked). Thread pool threads are usually hold available for short running actions.
  • There is no way to get notified if an error occurs within the endless loop. To avoid that, put the endless loop in a try catch block or add a ContinueWith for the failure case to the created task.
  • The class allows to start the ThreadMonitor multiple times. That is actually not desired, therefore I would throw an exception when trying to call Run twice.
  • Honestly, I don't understand how your "DeadLockPreventionLogic" works, but it looks kind of adventurous. Probably, the StackTrace may contain some interesting information, but it is also possible that the StackTrace belongs to unproblematic logic that was coincidentally executed when the StackTrace has be caught. Additional to logging the StackTrace, it makes definitely sense to log the entry points of all use workflows.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.