[Mono-devel-list] proposed patch for System.Configuration.ConfigurationSetting
eric lindvall
eric at 5stops.com
Thu Nov 20 13:15:50 EST 2003
inlined again.
On Thu, 20 Nov 2003, Gonzalo Paniagua Javier wrote:
> > public static DefaultConfig GetInstance ()
> > {
> > if (instance == null) {
> > lock (typeof (DefaultConfig)) {
> > if (instance == null) {
> > Interlocked.Exchange (
> > ref instance, new DefaultConfig());
> > }
> > }
> > }
> >
> > return ((DefaultConfig) instance);
> > }
> >
> > it does the first check so we don't need to lock every time we try to get
> > any configuration setting.
>
> Then we should remove the first "if"... I know we do that in many
> places, but see
> http://research.microsoft.com/~birrell/papers/ThreadsCSharp.pdf for
> reasons not to do that.
very interesting! i'll have to keep that in mind.
i realized there's no reason not to use the static constructor to do it..
it'll solve the problem and i don't have to have any locks.
> > > > > IMO, the changes to GetDocumentForSection are not needed. I vote for
> > > > > leaving the current code as it is.
> > > >
> > > > the filename check is now completely redundant, and there are cases (if
> > > > you get exceptions from within the method) where we will leak fd's if you
> > > > don't use a using() statement (or try/finally).
> > >
> > > If we leak fd's, then the GC is not doing its work. The GC will call the
> > > FileStream finalizer in case of an exception in between. We don't set
> > > the fd free inmediately, but well...
> >
> > the only other objection i have to the current implementation of
> > GetDocumentForSection() is the returning null instead of allowing the
> > exception to be thrown if you can't load the file.
>
> That change you did is probably the cause of that exception i reported
> in the previous message.
that's quite likely, but i can't seem to find a case that induces the
exception.
if you really want me to take out the GetDocumentForSection() changes, i
will.
> > > Okay. You should test xsp a bit when modifying this stuff, with and
> > > without web.config file.
> >
> > are there any specific tests you would suggest? i've tried renaming
> > web.config as well as xsp.exe.config and
>
> No, just running it and try 1 aspx + 1 asmx should be ok. Then try the
> same without web.config file.
I've ran a bunch of different aspx and asmx files with and without the
web.config.
-------------- next part --------------
Index: class/System/System.Configuration/ConfigurationSettings.cs
===================================================================
RCS file: /mono/mcs/class/System/System.Configuration/ConfigurationSettings.cs,v
retrieving revision 1.29
diff -u -p -u -r1.29 ConfigurationSettings.cs
--- class/System/System.Configuration/ConfigurationSettings.cs 17 Nov 2003 22:53:17 -0000 1.29
+++ class/System/System.Configuration/ConfigurationSettings.cs 20 Nov 2003 19:12:23 -0000
@@ -17,21 +17,19 @@ using System.IO;
using System.Runtime.CompilerServices;
using System.Xml;
using System.Xml.XPath;
+using System.Threading;
namespace System.Configuration
{
public sealed class ConfigurationSettings
{
- static IConfigurationSystem config;
-
private ConfigurationSettings ()
{
}
public static object GetConfig (string sectionName)
{
- if (config == null)
- config = DefaultConfig.GetInstance ();
+ IConfigurationSystem config = DefaultConfig.GetInstance ();
return config.GetConfig (sectionName);
}
@@ -39,14 +37,15 @@ namespace System.Configuration
public static NameValueCollection AppSettings
{
get {
- object appSettings = GetConfig ("appSettings");
+ NameValueCollection appSettings =
+ (NameValueCollection) GetConfig ("appSettings");
+
if (appSettings == null)
appSettings = new NameValueCollection ();
- return (NameValueCollection) appSettings;
+ return appSettings;
}
}
-
}
//
@@ -55,58 +54,62 @@ namespace System.Configuration
//
class DefaultConfig : IConfigurationSystem
{
- static object creatingInstance = new object ();
- static object buildingData = new object ();
static DefaultConfig instance;
ConfigurationData config;
+ static DefaultConfig()
+ {
+ instance = new DefaultConfig();
+ }
+
private DefaultConfig ()
{
+ this.Init();
}
public static DefaultConfig GetInstance ()
{
- if (instance == null) {
- lock (creatingInstance) {
- if (instance == null) {
- instance = new DefaultConfig ();
- instance.Init ();
- }
-
- }
- }
-
- return instance;
+ return (instance);
}
public object GetConfig (string sectionName)
{
- if (config == null)
- return null;
+ if (config == null)
+ {
+ throw (new InvalidOperationException ("ConfigurationSettings not initialized."));
+ }
return config.GetConfig (sectionName);
}
public void Init ()
{
- if (config == null){
- lock (buildingData) {
- if (config != null)
- return;
-
- ConfigurationData data = new ConfigurationData ();
- if (data.Load (GetMachineConfigPath ())) {
- ConfigurationData appData = new ConfigurationData (data);
- appData.Load (GetAppConfigPath ());
- config = appData;
- } else {
- Console.WriteLine ("** Warning **: cannot find " + GetMachineConfigPath ());
- Console.WriteLine ("Trying to load app config file...");
- data.Load (GetAppConfigPath ());
- config = data;
- }
- }
- }
+ try
+ {
+ ConfigurationData data = new ConfigurationData (GetMachineConfigPath ());
+
+ try
+ {
+ // try to load the app data.
+ ConfigurationData appData = new ConfigurationData (GetAppConfigPath (), data);
+ config = appData;
+ }
+ catch
+ {
+ // if we couldn't load the app data,
+ // we won't chain it
+ config = data;
+ }
+ }
+ catch (Exception e)
+ {
+ Console.WriteLine ("** Warning **: cannot find " +
+ GetMachineConfigPath () + ": " + e.Message);
+ Console.WriteLine ("Trying to load app config file...");
+
+ ConfigurationData data = new ConfigurationData (GetAppConfigPath ());
+ config = data;
+ }
}
[MethodImplAttribute(MethodImplOptions.InternalCall)]
@@ -126,7 +129,6 @@ namespace System.Configuration
return null;
return configFile;
-
}
}
@@ -188,56 +190,31 @@ namespace System.Configuration
static object removedMark = new object ();
static object groupMark = new object ();
static object emptyMark = new object ();
- FileWatcherCache fileCache = null;
-
- private FileWatcherCache FileCache {
- get {
- if (fileCache == null) {
- if (fileName != null) {
- fileCache = new FileWatcherCache (fileName);
- } else {
- fileCache = parent.FileCache;
- }
- }
+ FileWatcherCache fileCache;
- return fileCache;
- }
- }
-
- public ConfigurationData () : this (null)
+ public ConfigurationData (string fileName) : this (fileName, null)
{
}
- public ConfigurationData (ConfigurationData parent)
+ public ConfigurationData (string fileName, ConfigurationData parent)
{
- this.parent = (parent == this) ? null : parent;
- factories = new Hashtable ();
- }
-
- public bool Load (string fileName)
- {
- if (fileName == null || !File.Exists (fileName))
- return false;
+ if (fileName == null)
+ {
+ throw (new ArgumentNullException ("fileName"));
+ }
this.fileName = fileName;
- XmlTextReader reader = null;
+ this.fileCache = new FileWatcherCache (fileName);
+ this.parent = (parent == this) ? null : parent;
+ factories = new Hashtable ();
- try {
- try {
- FileStream fs = new FileStream (fileName, FileMode.Open, FileAccess.Read);
- reader = new XmlTextReader (fs);
- } catch (Exception) {
- return false;
- }
+ using (FileStream fs = new FileStream (fileName, FileMode.Open, FileAccess.Read))
+ {
+ XmlTextReader reader = new XmlTextReader (fs);
InitRead (reader);
ReadConfigFile (reader);
- } finally {
- if (reader != null)
- reader.Close();
- }
-
- return true;
+ }
}
object GetHandler (string sectionName)
@@ -246,8 +223,8 @@ namespace System.Configuration
if (o == null || o == removedMark) {
if (parent != null)
return parent.GetHandler (sectionName);
-
- return null;
+ else
+ return null;
}
if (o is IConfigurationSectionHandler)
@@ -271,50 +248,44 @@ namespace System.Configuration
}
//TODO: Should use XPath when it works properly for this.
- XmlDocument GetDocumentForSection (string sectionName)
+ private XmlDocument GetDocumentForSection (string sectionName)
{
- if (fileName == null || !File.Exists (fileName))
- return null;
-
- XmlTextReader reader = null;
- try {
- FileStream fs = new FileStream (fileName, FileMode.Open, FileAccess.Read);
- reader = new XmlTextReader (fs);
- } catch {
- return null;
- }
-
- ConfigXmlDocument doc = new ConfigXmlDocument ();
- InitRead (reader);
- string [] sectionPath = sectionName.Split ('/');
- int i = 0;
- if (!reader.EOF) {
- if (reader.Name == "configSections")
- reader.Skip ();
-
- while (!reader.EOF) {
- if (reader.NodeType == XmlNodeType.Element &&
- reader.Name == sectionPath [i]) {
- if (++i == sectionPath.Length) {
- doc.LoadSingleElement (fileName, reader);
- break;
- }
- MoveToNextElement (reader);
- continue;
- }
- reader.Skip ();
- if (reader.NodeType != XmlNodeType.Element)
- MoveToNextElement (reader);
- }
- }
+ using (FileStream fs = new FileStream (fileName, FileMode.Open, FileAccess.Read))
+ {
+ XmlTextReader reader = new XmlTextReader (fs);
+
+ ConfigXmlDocument doc = new ConfigXmlDocument ();
+ InitRead (reader);
+ string [] sectionPath = sectionName.Split ('/');
+ int i = 0;
+ if (!reader.EOF) {
+ if (reader.Name == "configSections")
+ reader.Skip ();
+
+ while (!reader.EOF) {
+ if (reader.NodeType == XmlNodeType.Element &&
+ reader.Name == sectionPath [i]) {
+ if (++i == sectionPath.Length) {
+ doc.LoadSingleElement (fileName, reader);
+ break;
+ }
+ MoveToNextElement (reader);
+ continue;
+ }
+ reader.Skip ();
+ if (reader.NodeType != XmlNodeType.Element)
+ MoveToNextElement (reader);
+ }
+ }
- reader.Close ();
- return doc;
+ return doc;
+ }
}
- object GetConfigInternal (string sectionName)
+ private object GetConfigInternal (string sectionName)
{
object handler = GetHandler (sectionName);
+
if (handler == null)
return null;
@@ -322,6 +293,7 @@ namespace System.Configuration
return handler;
object parentConfig = null;
+
if (parent != null)
parentConfig = parent.GetConfig (sectionName);
@@ -329,8 +301,8 @@ namespace System.Configuration
if (doc == null || doc.DocumentElement == null) {
if (parentConfig == null)
return null;
-
- return parentConfig;
+ else
+ return parentConfig;
}
return ((IConfigurationSectionHandler) handler).Create (parentConfig, fileName, doc.DocumentElement);
@@ -338,17 +310,20 @@ namespace System.Configuration
public object GetConfig (string sectionName)
{
- object config = this.FileCache [sectionName];
+ lock (this)
+ {
+ object config = this.fileCache [sectionName];
- if (config == emptyMark)
- return null;
+ if (config == emptyMark)
+ return null;
- if (config != null)
- return config;
+ if (config != null)
+ return config;
- config = GetConfigInternal (sectionName);
- this.FileCache [sectionName] = (config == null) ? emptyMark : config;
- return config;
+ config = GetConfigInternal (sectionName);
+ this.fileCache [sectionName] = (config == null) ? emptyMark : config;
+ return config;
+ }
}
private object LookForFactory (string key)
More information about the Mono-devel-list
mailing list