上一篇文章介绍了一个程序崩溃的调试过程。经过细致的分析,我们发现程序崩溃的原因是CLR的缺陷。那么,有没有可能修改产品代码,来避免崩溃呢?
检查崩溃线程的托管栈可知,产品代码是在查询数据库。
0:013> !clrstack
OS Thread Id: 0x2630 (13)
Child-SP RetAddr Call Site
000000001d3cd2e0 000007fef48dd8ec System.Data.SqlClient.SqlCachedStream.get_TotalLength()
000000001d3cd330 000007fef723a453 System.Data.SqlTypes.SqlXmlStreamWrapper.get_Length()
000000001d3cd370 000007fef74a12d8 System.Xml.XmlReader.CalcBufferSize(System.IO.Stream)
000000001d3cd3b0 000007fef980d502 System.Xml.XmlReader.CreateSqlReader(System.IO.Stream,
System.Xml.XmlReaderSettings, System.Xml.XmlParserContext)
000000001d3cdcf0 000007fef892ae46 System.Reflection.RuntimeMethodInfo.Invoke
(System.Object, System.Reflection.BindingFlags, System.Reflection.Binder,
System.Object[], System.Globalization.CultureInfo, Boolean)
000000001d3cde90 000007fef4704211 System.Reflection.RuntimeMethodInfo.Invoke
(System.Object, System.Reflection.BindingFlags, System.Reflection.Binder,
System.Object[], System.Globalization.CultureInfo)
000000001d3cdee0 000007fef4703fc2 System.Data.SqlTypes.SqlXml.CreateReader()
000000001d3cdf60 000007fef4a010db System.Data.SqlTypes.SqlXml.get_Value()
000000001d3cdfb0 000007fef43fe396 System.Data.SqlClient.SqlBuffer.get_Value()
000000001d3ce080 000007fef43fe5a2 System.Data.SqlClient.SqlDataReader.GetValueInternal
(Int32)
000000001d3ce0d0 000007fef4419d37 System.Data.SqlClient.SqlDataReader.GetValues
(System.Object[])
000000001d3ce150 000007fef4419c47 System.Data.ProviderBase.DataReaderContainer
+CommonLanguageSubsetDataReader.GetValues(System.Object[])
000000001d3ce180 000007fef4411883 System.Data.ProviderBase.SchemaMapping.LoadDataRow()
000000001d3ce1d0 000007fef4411727 System.Data.Common.DataAdapter.FillLoadDataRow
(System.Data.ProviderBase.SchemaMapping)
000000001d3ce260 000007fef4411584 System.Data.Common.DataAdapter.FillFromReader
(System.Data.DataSet, System.Data.DataTable, System.String,
System.Data.ProviderBase.DataReaderContainer, Int32,
Int32, System.Data.DataColumn, System.Object)
000000001d3ce330 000007fef441266d System.Data.Common.DataAdapter.Fill(System.Data.DataSet,
System.String, System.Data.IDataReader, Int32, Int32)
000000001d3ce3f0 000007fef44124fd System.Data.Common.DbDataAdapter.FillInternal
(System.Data.DataSet, System.Data.DataTable[], Int32, Int32, System.String,
System.Data.IDbCommand, System.Data.CommandBehavior)
000000001d3ce4b0 000007fef4412266 System.Data.Common.DbDataAdapter.Fill
(System.Data.DataSet, Int32, Int32, System.String,
System.Data.IDbCommand, System.Data.CommandBehavior)
000000001d3ce560 000007ff001ab365 System.Data.Common.DbDataAdapter.Fill
(System.Data.DataSet)
000000001d3ce5f0 000007ff001aaca3 MyApp.DAL.DatabaseHelper.ExecuteDataSet(System.String,
System.Data.CommandType)
000000001d3ce660 000007ff001a99ad MyApp.DAL.ReportDAO.GetPendingAndRunningReportJobs()
000000001d3ce6c0 000007fef88cdd38 MyApp.Report.CheckJobStatus(System.Object)
000000001d3ce930 000007fef980d502 System.Threading.ExecutionContext.runTryCode
(System.Object)
000000001d3cf1e0 000007fef890eb56 System.Threading.ExecutionContext.Run
(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
000000001d3cf230 000007fef980d502 System.Threading._TimerCallback.PerformTimerCallback
(System.Object)
函数CheckJobStatus旨在检查数据库中Job的状态,它调用数据抽象层(DAL)的函数GetPendingAndRunningReportJobs。后者调用函数ExecuteDataSet,返回所有等待(pending)和运行(running)的Job。它们的代码如下。
public static DataSet GetPendingAndRunningReportJobs()
{
//Status 0 = Not Generated, 1 = pending, 2 = Running
const String query = "select * from report with(nolock) where [Status] in (0, 1, 2)";
using (var db = new DatabaseHelper())
{
DataSet result = db.ExecuteDataSet(query);if (result != null && result.Tables.Count > 0 && result.Tables[0].Rows.Count > 0)
{
return result;
}
return null;
}
}public DataSet ExecuteDataSet(String query, CommandType commandtype)
{
DbDataAdapter adapter = objFactory.CreateDataAdapter();
objCommand.CommandText = query;
objCommand.CommandType = commandtype;
adapter.SelectCommand = objCommand;
DataSet ds = new DataSet();
ds.Locale = CultureInfo.InvariantCulture;
try
{
adapter.Fill(ds);
return ds;
}
catch(SqlException e)
{
throw TranslateException(e);
}
finally
{
objCommand.Parameters.Clear();
adapter.Dispose();
}
}
作为通用的辅助函数,ExecuteDataSet没有修改的空间。在GetPendingAndRunningReportJobs中,有一条语句是问题所在。
const String query = "select * from Job with(nolock) where [Status] in (0, 1, 2)";
该语句存在以下问题。
- 违反了基本的编程规则:不要在产品代码中使用非限定性查询"select * from …"。
- 在业务代码中硬编码查询,使得业务代码与持久层紧耦合。而非限定性查询使得耦合更加隐晦,为产品演化引入了风险。
- 非限定性查询返回了所有列。对于CheckJobStatus而言,许多列是不需要的。这样做引入了不必要的性能开销。
- 从托管栈可知,崩溃发生在读取XML数据的过程中。在表Job中,只有一列的数据类型是XML,它存放了Job的定义。然而CheckJobStatus并不需要Job的定义。也就是说,读取无用数据导致了程序崩溃。
我将以上情况反映给程序的开发人员。由于临近发布,我们决定不做大的修改,只是将查询字符串修改为:
const String query = "select Result, ReportId, Status from Job with(nolock) where [Status] in (0, 1, 2)";
开发人员改完代码后,提供了一份私有版本(private build)给我,让我在预发布环境中测试,以检查该修改确实能避免崩溃。所谓私有版本只是一个修改后的DLL(托管程序集),我用它替换了有问题的DLL,并重启了服务。
问题发现在周五上午。当我替换完DLL,已经是傍晚。考虑到该问题的重现可能需要几天的时间(在崩溃之前,服务运行了近一个星期),我待服务重启完毕,未作仔细检查,就去吃晚饭了。晚饭之后,我打算检查一下系统状态,就回家欢度周末。将Windbg附加到被测试服务,猛然发现有许多异常。原来开发人员有一个笔误,他在查询字符串中多写了一个逗号:
const String query = "select Result, ReportId, Status, from Job with(nolock) where [Status] in (0, 1, 2)";
恰恰是这个多余的逗号使得GetPendingAndRunningReportJobs的每一次执行都会抛出异常。这导致引发崩溃的代码路径(code path)无法被测试覆盖,验证私有版本的任务不能继续。
为什么GetPendingAndRunningReportJobs抛出异常,服务还能继续?这是因为开发人员注册了一个全局的异常处理函数,它吞没了所有的托管异常。虽然CheckJobStatus被定时调用,它所导致的异常并不会使服务崩溃。
我急忙再去找开发人员,发现他已经先我一步去欢度周末了。我原想回滚这次更新,但是他的私有版本中还含有另一个重要修正。如果回滚到旧版本,就不能利用周末的时间来压力测试这个修正了。于是,我决定保持当前状态,等到周一再继续测试。
回顾这一天的情况,我有如下思考。
- 要遵循基本的编程规则,例如不在产品代码中硬编码查询,不使用非限定性查询等。遵循这些规则可以提高正确性,可维护性,避免不必要的性能损失和莫名的崩溃。
- 不要吞没异常。特别是,不要在全局的缺陷处理函数中,不区分具体情况就吞没全部异常。Andrew Hunt和David Thomas说:要崩溃,不要破坏(trash)。当”不可能情况“(例如SQL查询抛出语法异常)出现时,程序应该崩溃。
- 如果确实要吞没异常,应该详细记录异常细节,并利用邮件、系统日志、管理员页面等手段通知运营和开发团队。
- 在替换私有版本后,应该立即做必要的测试。要尽快发现严重的问题。
- 周五傍晚是一个奇妙的时段,不要做高风险操作。
到了周一,我向开发人员反映问题,重新获得了一份私有版本的DLL。经过一周的压力测试,服务没有再崩溃。说明这次的修改是有效的。