2
\$\begingroup\$

I'm trying to pass my logger to a Logging Aspect that I've created with PostSharp. I have one logger per job run (because I need to send each job's logs to different folders), each job runs in different threads using BackgroundWorker. This is the code that I have:

[Serializable]
public class LogExecutionTimeAttribute : OnMethodBoundaryAspect
{
    [NonSerialized]
    public static Func<ILog> GetLogger;

    private string methodName;

    public override void CompileTimeInitialize(MethodBase method, AspectInfo aspectInfo)
    {
        this.methodName = method.Name;
    }

    public override void OnEntry(MethodExecutionArgs args)
    {
        args.MethodExecutionTag = Stopwatch.StartNew();
        base.OnEntry(args);
    }

    public override void OnExit(MethodExecutionArgs args)
    {
        Stopwatch sw = (Stopwatch)args.MethodExecutionTag;
        sw.Stop();
        var logger = GetLogger();
        logger.DebugFormat(
            "{0} executed in {1} seconds.", 
            this.methodName, 
            sw.ElapsedMilliseconds / 1000.0);
        base.OnExit(args);
    }
}

Each thread would do:

var log = CreateLoggerForJobId(jobId);
LogExecutionTimeAttribute.GetLogger = () => log;

I'm doubtful about the Func<ILog> GetLogger being static - How will it work when called from multiple threads? My manual tests seem to suggest that this is working, but I'm not sure if this is thread safe. I am also not sure if using NonSerialized attribute on GetLogger helps.

Entire working code is here:

https://gist.github.com/rnarayana/5518327f9c7c94f8c2fde4ae7fee7733

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

You're reading and writing the GetLogger static field on multiple threads simultaneously, so the shown code is not thread-safe. You can mark the field with the [ThreadStatic] attribute to have a unique value on each thread.

Also, you don't need to mark the field as [NonSerialized] because static fields are not serialized anyway (https://stackoverflow.com/questions/3903079/does-default-serialization-in-c-sharp-serialize-static-fields).

\$\endgroup\$
2
  • \$\begingroup\$ Thanks, do you know how to achieve what I want? \$\endgroup\$
    – Narayana
    Sep 1, 2016 at 17:12
  • \$\begingroup\$ If you apply[ThreadStatic] to your logger field, then the code should be correct. However, that's still not the best design, because the logger aspect class normally should not depend on whether you have one logger per thread, one logger per instance, etc. You can read more info about passing dependencies into aspects in docs: doc.postsharp.net/consuming-dependencies. Most often you would choose to use service locator or import logger property from the target type. \$\endgroup\$
    – AlexD
    Sep 2, 2016 at 9:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.