[Mono-devel-list] proposed patch for System.Configuration.ConfigurationSetting

[eric lindvall]

inlined again.

On Thu, 20 Nov 2003, Gonzalo Paniagua Javier wrote:

> > > There's no need to use Interlocked here, as we're already inside a lock.
> > 
> > isn't it going to be possible that the first "instance == null" check
> > could find a half-assigned object ref? i debated that one for a bit, but
> > it seems it is "more correct" to do the exchange.
> 
> You're already inside the lock. That could happen if you do:
>         if (instace != null)
>         	return instace;
>         lock (typeof (DefaultConfig) {
>         	blah
>         }


the full method is:

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.


> > > 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.

> > > Also if I try to run an application that uses ConfigurationSettings and
> > > there's no application configuration file (ie, only machine.config is
> > > read), i get an exception:
> > > 
> > > Unhandled Exception: System.IO.FileNotFoundException: Could not find
> > > file "/home/gpanjav/go-mono/appsettings.exe.config"
> > > 
> > > This should not happen.
> > 
> > correct. i'm not sure why i didn't find that when i was running my tests
> > (i didn't have an app.config file either).
> 
> 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