Thread.Abort causing deadlock in catch{} statement

2.3k views Asked by At

I have a set of threaded classes that print different types of documents. The classes use inheritance to share common code. The class constructor requires file name and printer name arguments. A Print() method creates a new worker thread, waits for the worker thread to complete using Thread.Join(timeout) and calls Thread.Abort() on the worker thread if the Join times out. The worker thread starts an application that can open the specified file, causes the file to be sent to printer synchronously (usually using application's Print method) and exits. The worker thread's code is wrapped in a try{} ... catch{} block to deal with any unforeseen crashes of the external application. The catch block contains minimal cleanup and logging.

    internal static FilePackage TryPrintDocumentToPdf(string Filename)
    {
                .....

                Logging.Log("Printing this file using PowerPoint.", Logging.LogLevel.Debug);
                printableFormat = true;

                fc = new FileCollector(Email2Pdf.Settings.Printer.PdfAttachmentCollectDirectoryObj, FileCollector.CollectMethods.FileCount | FileCollector.CollectMethods.FilesNotInUse | FileCollector.CollectMethods.ProcessExit);
                fc.FileCount = 1;
                fc.ProcessNames = new string[] { OfficePowerPointExe, Email2Pdf.Settings.Printer.PrinterExe };
                fc.Prepare();

                using (PowerPointPrinter printer = new PowerPointPrinter(Filename, Email2Pdf.Settings.Printer.PdfAttachmentPrinter))
                {
                    printer.KillApplicationOnClose = true;
                    printer.Print();
                    printOk = printer.PrintOk;
                }

                .....
    }

    internal abstract class ApplicationPrinter : IDisposable
    {
        protected abstract string applicationName { get; }

        protected string filename;
        protected string printer;

        protected bool workerPrintOk;
        protected bool printOk;
        public bool PrintOk { get { return printOk; } }
        public bool KillApplicationOnClose { get; set; }

        public void Print()
        {
            System.Threading.Thread worker = new System.Threading.Thread(printWorker);
            DateTime time = DateTime.Now;
            worker.Start();

            if (worker.Join(new TimeSpan(0, Email2Pdf.Settings.Printer.FileGenerateTimeOutMins, 0)))
            {
                printOk = workerPrintOk;
            }
            else
            {
                worker.Abort();
                printOk = false;
                Logging.Log("Timed out waiting for " + applicationName + " file " + filename + " to print.", Logging.LogLevel.Error);
            }
        }

        protected abstract void Close();
        protected abstract void printWorker();

        public virtual void Dispose() { Close(); }
    }
    internal class PowerPointPrinter : ApplicationPrinter
    {
        private const string appName = "PowerPoint";
        protected override string applicationName { get { return appName; } }
        private Microsoft.Office.Interop.PowerPoint.Application officePowerPoint = null;

        public PowerPointPrinter(string Filename, string Printer)
        {
            filename = Filename;
            printer = Printer;
            this.Dispose();
        }

        protected override void printWorker()
        {
            try
            {
                officePowerPoint = new Microsoft.Office.Interop.PowerPoint.Application();
                officePowerPoint.DisplayAlerts = Microsoft.Office.Interop.PowerPoint.PpAlertLevel.ppAlertsNone;

                Microsoft.Office.Interop.PowerPoint.Presentation doc = null;

                doc = officePowerPoint.Presentations.Open(
                    filename,
                    Microsoft.Office.Core.MsoTriState.msoTrue,
                    Microsoft.Office.Core.MsoTriState.msoFalse,
                    Microsoft.Office.Core.MsoTriState.msoFalse);
                doc.PrintOptions.ActivePrinter = printer;
                doc.PrintOptions.PrintInBackground = Microsoft.Office.Core.MsoTriState.msoFalse;
                doc.PrintOptions.OutputType = Microsoft.Office.Interop.PowerPoint.PpPrintOutputType.ppPrintOutputSlides;
                doc.PrintOut();

                System.Threading.Thread.Sleep(500);

                doc.Close();
                //Marshal.FinalReleaseComObject(doc);
                doc = null;

                workerPrintOk = true;
            }
            catch (System.Exception ex)
            {
                Logging.Log("Unable to print PowerPoint file " + filename + ". Exception: " + ex.Message, Logging.LogLevel.Error);
                Close();
                workerPrintOk = false;
            }
        }

        protected override void Close()
        {
            try
            {
                if (officePowerPoint != null)
                    officePowerPoint.Quit();
                Marshal.FinalReleaseComObject(officePowerPoint);
                officePowerPoint = null;
                if (KillApplicationOnClose)
                    Utility.KillProcessesByName(OfficePowerPointExe);
            }
            catch { }
        }
    }

I found my application non-responsive, with the main thread in a Sleep/Wait/Join at the Thread.Abort() line. I do not recall the status of the worker thread, but the logging that was supposed to be performed in the catch{} block did not take place. (I Attached to my process with VS2010 after I found it non-responsive).

I refer to the following Note from the Thread.Abort Method:

The thread that calls Abort might block if the thread that is being aborted is in a protected region of code, such as a catch block, finally block, or constrained execution region. If the thread that calls Abort holds a lock that the aborted thread requires, a deadlock can occur.

I believe I have a dead-locking issue because (1) it does not always happen, and (2) because of the Note on MSDN (above).

  1. The Note appears to suggest that using try{} ... catch{} is NEVER safe inside a thread if the thread can be Abort()'ed. Is this true?
  2. I do not see how I can avoid using Abort() in my scenario. Will using Thread.Interrupt() instead make any difference?
  3. How to I fix the dead-locking issue I have?

BackgroundWorker does not work for me because I do not need progress reporting and, more importantly, it is possible that my worker thread will block indefinitely when it executes third party applications. For the same reason, I cannot ask my thread to terminate, but have one option only - to ruthlessly Abort() the worker thread.

3

There are 3 answers

0
Serge On BEST ANSWER

I think I found a solution by making the following changes:

  1. Do not call Thread.Abort() if we know that the worker thread is executing a catch{} block (see protected volatile bool isPrinting below).
  2. Use a separate thread to call Thread.Abort() and encourage a context switch with Sleep(0) (see private void AbortPrintWorker() below).

    internal abstract class ApplicationPrinter : IDisposable
    {
        protected abstract string applicationName { get; }
    
        protected string filename;
        protected string printer;
    
        protected bool workerPrintOk;
        protected bool printOk;
        public bool PrintOk { get { return printOk; } }
        public bool KillApplicationOnClose { get; set; }
    
        protected System.Threading.Thread worker;
        protected volatile bool isPrinting;
    
        public void Print()
        {
            worker = new System.Threading.Thread(printWorker);
            DateTime time = DateTime.Now;
            worker.Start();
    
            if (worker.Join(new TimeSpan(0, Email2Pdf.Settings.Printer.FileGenerateTimeOutMins, 0)))
            {
                printOk = workerPrintOk;
            }
            else
            {
                AbortPrintWorker();
                printOk = false;
                Logging.Log("Timed out waiting for " + applicationName + " file " + filename + " to print.", Logging.LogLevel.Error);
            }
        }
        protected abstract void printWorker();
    
        public abstract void Dispose();
    
        private void AbortPrintWorker()
        {
            System.Threading.Thread abortThread = new System.Threading.Thread(abortWorker);
            if (isPrinting)
            {
                abortThread.Start();
                System.Threading.Thread.Sleep(0);
                abortThread.Join();
            }
            else
            {
                worker.Join();
            }
        }
    
        private void abortWorker()
        {
            worker.Abort();
            worker.Join();
        }
    }
    
    internal class PowerPointPrinter : ApplicationPrinter
    {
        private const string appName = "PowerPoint";
        protected override string applicationName { get { return appName; } }
        private Microsoft.Office.Interop.PowerPoint.Application officePowerPoint = null;
    
        public PowerPointPrinter(string Filename, string Printer)
        {
            filename = Filename;
            printer = Printer;
            this.Dispose();
        }
    
        protected override void printWorker()
        {
            try
            {
                isPrinting = true;
    
                officePowerPoint = new Microsoft.Office.Interop.PowerPoint.Application();
                officePowerPoint.DisplayAlerts = Microsoft.Office.Interop.PowerPoint.PpAlertLevel.ppAlertsNone;
    
                Microsoft.Office.Interop.PowerPoint.Presentation doc = null;
    
                doc = officePowerPoint.Presentations.Open(
                    filename,
                    Microsoft.Office.Core.MsoTriState.msoTrue,
                    Microsoft.Office.Core.MsoTriState.msoFalse,
                    Microsoft.Office.Core.MsoTriState.msoFalse);
                doc.PrintOptions.ActivePrinter = printer;
                doc.PrintOptions.PrintInBackground = Microsoft.Office.Core.MsoTriState.msoFalse;
                doc.PrintOptions.OutputType = Microsoft.Office.Interop.PowerPoint.PpPrintOutputType.ppPrintOutputSlides;
                doc.PrintOut();
    
                System.Threading.Thread.Sleep(500);
    
                doc.Close();
                Marshal.FinalReleaseComObject(doc);
                doc = null;
    
                workerPrintOk = true;
    
                isPrinting = true;
            }
            catch (System.Exception ex)
            {
                isPrinting = false;
    
                Logging.Log("Unable to print PowerPoint file " + filename + ". Exception: " + ex.Message, Logging.LogLevel.Error);
                workerPrintOk = false;
            }
        }
    
        public override void Dispose()
        {
            try
            {
                if (officePowerPoint != null)
                    officePowerPoint.Quit();
                Marshal.FinalReleaseComObject(officePowerPoint);
                officePowerPoint = null;
                if (KillApplicationOnClose)
                    Utility.KillProcessesByName(OfficePowerPointExe);
            }
            catch { }
        }
    }
    

AbortPrintWorker() creates a separate thread to call Abort() on the worker thread. I believe this deals with the issue highlighted in the Note on Abort():

The thread that calls Abort might block if the thread that is being aborted is in a protected region of code, such as a catch block, finally block, or constrained execution region. If the thread that calls Abort holds a lock that the aborted thread requires, a deadlock can occur.

Is this correct?

3
Mitch Wheat On

Your mechanism using Thread.Abort() is not a good one. In fact, calling Thread.Abort() should be avoided.

The thread that calls Abort might block if the thread that is being aborted is in a protected region of code, such as a catch block, finally block, or constrained execution region. If the thread that calls Abort holds a lock that the aborted thread requires, a deadlock can occur. Ref.

Instead, use a BackgroundWorker which supports cancellation, progress reporting (and auto marshalling onto UI thread in completed event).

1
Brian Rogers On

It looks to me like you are basically remote-controlling the PowerPoint application in order to print a PowerPoint document. Thus you could be subject to any dialog boxes that the application put up (or tried to put up) on the screen. If this whole thing is being run in the background (e.g. on a server), there may not be a user to dismiss any such dialogs, so that could explain part of the issue. My recommendation would be to look into third-party libraries that would allow you to load a PPT file and print it (or convert it to PDF and print that) without having to rely on the PowerPoint application. Then you wouldn't have to wait on an external app outside your control and you woudln't have to resort to forcefully aborting threads.