I've read Richter's article, and I think that the security concerns
don't hold much weight, to be honest. If it got to the point where
malicious code could run on your machine, then whether or not you expose the
lock your objects use (namely, this), doesn't matter. The malicious code
could simply perform a DoS attack using:
while (true);
Now the argument that you only want to lock on a subset of the members
of your class holds more weight, but the counterpoint to that is that with
proper design (where you don't have super sized classes), your operations
will affect the whole class anyways (because they will not be loaded
tremendously with properties and expand the scope of the class by too much).
Also, with reflection, it's almost impossible to truly keep someone from
accessing your locks. You could remove permissions for reflection from the
class, I believe, but then you are hindering yourself in other ways. You
have to decide where to trade off.
--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)
Mr. Mountain said:
Hi Nicholas,
If I understand correctly, using the MethodImpl attribute is the same as a
lock on 'this'.
Locking on 'this' isn't recommended according to an excellent article by
Jeffrey Richter at
http://msdn.microsoft.com/msdnmag/issues/03/01/NET/
Jon Skeet also makes some excellent points about what to lock on at
http://www.yoda.arachsys.com/csharp/threads/lockchoice.shtml
Bottom line: create a private variable to lock on and use it only for
synchronization purposes.
Regards,
Mountain
in
message news:%
[email protected]...
Dan,
I wouldn't use a Mutex. I would just make the method synchronized, like
so:
[MethodImpl(MethodImplOptions.Synchronized)]
public override string MapMessage ( string messageSource )
{
// create and load up a new transform if required
mut.WaitOne();
if ( xslt == null )
{
if ( File.Exists(configFile) )
{
xslt = new XslTransform();
xslt.Load( configFile );
}
}
else
{
throw ( new ApplicationException ( "Configuration error, XSL file
does not exist. " + configFile));
}
// rest of the function code here
}
Hope this helps.
--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)
"Dan Bass" <danielbass [at] postmaster [dot] co [dot] uk> wrote in
message
I know that is wrong! Just realised the problem it could give me...
What about this?
XslTransform xslt = null;
Mutex mut = new Mutex();
public override string MapMessage ( string messageSource )
{
//
// create and load up a new transform if required
mut.WaitOne();
if ( xslt == null )
{
if ( File.Exists(configFile) )
{
xslt = new XslTransform();
xslt.Load( configFile );
}
else
{
mut.ReleaseMutex();
throw ( new ApplicationException ( "Configuration
error, XSL file does not exist. " + configFile ) );
}
}
mut.ReleaseMutex();
//
// rest of the function code here
}
"Dan Bass" <danielbass [at] postmaster [dot] co [dot] uk> wrote in message
I know that XslTransform's Transform is thread safe according to the
MSDN, and that Load is not. I've therefore applied this simply Mutex
to
it and would just like to confirm this is okay.
XslTransform xslt = null;
Mutex mut = new Mutex();
public override string MapMessage ( string messageSource )
{
//
// create and load up a new transform if required
if ( xslt == null )
{
if ( File.Exists(configFile) )
{
mut.WaitOne();
xslt = new XslTransform();
xslt.Load( configFile );
mut.ReleaseMutex();
}
else
{
throw ( new ApplicationException ( "Configuration
error, XSL file does not exist. " + configFile ) );
}
}
//
// rest of the function code here
}