NullReferenceException from SharpVectors.Renderers.Wpf.WpfSvgRendering.AdjustViewbox()

[atmgrifter00]

In the most recent nuget package (1.4.0), we can hit the following exception:

System.NullReferenceException: Object reference not set to an instance of an object.
   at SharpVectors.Renderers.Wpf.WpfSvgRendering.AdjustViewbox()
   at SharpVectors.Renderers.Wpf.WpfSvgRendering.AfterRender(WpfDrawingRenderer renderer)
   at SharpVectors.Renderers.Wpf.WpfRenderingHelper.RenderElement(ISvgElement svgElement)
   at SharpVectors.Renderers.Wpf.WpfRenderingHelper.Render(ISvgDocument docElement)
   at SharpVectors.Renderers.Wpf.WpfDrawingRenderer.Render(ISvgDocument node)
   at SharpVectors.Converters.FileSvgReader.LoadFile(TextReader textReader)

Now, I haven't completely wrapped my head around what might be the root cause, but if I were to guess it would be that _context is null here. Looking at how _context is supposed to be initialized it's hard to see how it would ever be not null, as the public constructor for WpfSvgRendering only offers the one argument for initialization which will ultimately pass null to the base constructor to initialize _context with.

[paulushub]

Looking at how _context is supposed to be initialized it's hard to see how it would ever be not null,
as the public constructor for WpfSvgRendering only offers the one argument for initialization
which will ultimately pass null to the base constructor to initialize _context with.

True, but the normal procedure will involve BeforeRender, Render and AfterRender calls, and the call to the base.BeforeRender will normally initialize the _context object as below.

public virtual void BeforeRender(WpfDrawingRenderer renderer)
{
    if (renderer == null)
    {
        return;
    }
    _isReady = false;
    _context = renderer.Context;

    if (_svgElement != null && _context != null)
    {
        if (string.IsNullOrWhiteSpace(_uniqueId))
        {
            _uniqueId = _svgElement.UniqueId;
        }
        _paintContext = new WpfSvgPaintContext(_uniqueId);
        _context.RegisterPaintContext(_paintContext);
    }
}

Looking at your stack, it seems the BeforeRender method is not called in this case. I am looking into it.

[atmgrifter00]

Ah, I see. If it helps, this is the code snippet that I believe is responsible for the noted exception call stack:

private void UpdateDrawing()
{
   var wpfSettings = new WpfDrawingSettings();
   wpfSettings.CultureInfo = wpfSettings.NeutralCultureInfo;
   using (var textReader = new StringReader(ImageData))
   {
       using (var fileReader = new FileSvgReader(wpfSettings))
       {
           fileReader.SaveXaml = false;
           fileReader.SaveZaml = false;
           var drawing = fileReader.Read(textReader);
           drawing.Freeze();
           Drawing = drawing;
        }
    }
}

Now, the object that holds on to this method may have been created off the UI-thread, but this doesn't seem inherently problematic, since the drawing instance is frozen.

[astarche]

It's actually _drawGroup that's null. You can only hit this if you're running the above code in parallel.

Here's the test I use that reproduces ~90% of the time:

const int NumberOfConsumers = 8;
var queue = new ConcurrentQueue<string>();

for (int i = 0; i < 200; ++i)
{
    queue.Enqueue(Image1);
}

var allTasks = new List<Task>();

for (int i = 0; i < NumberOfConsumers; ++i)
{
    allTasks.Add(Task.Run(() =>
    {
        while (queue.TryDequeue(out string imageData))
        {
            UpdateDrawing(imageData);
        }
    }));
}
await Task.WhenAll(allTasks);

I think the issue is with the _cacheRendering dictionary in WpfRendering.Create. When I see the exception, there are 2 threads that each have different elements with the same LocalName ("svg"). That code will give each of them the same WpfSvgRenderingNode, which they then Render on in parallel. This exposes that NullReferenceException if one thread clears _drawGroup with the call to Initialize while the other thread is between the null-check and dereference of _drawGroup in AdjustViewbox.

[paulushub]

I think the issue is with the _cacheRendering dictionary in WpfRendering.Create.

That was my initial thought, since that is the most unique change in the release 1.4. Thanks for investigating this issue.

[atmgrifter00]

I'd also like point out that we seem to be able to hit a dead-lock from essentially the same code path (being called from two separate threads). The call stacks in the two threads looked like this:

[paulushub]

@astarche @atmgrifter00
I have currently resolved both issues and still testing for any further thread safely issue. Your sample code helped a lot, thanks.

[paulushub]

@astarche @atmgrifter00
I have packed the above codes in WpfTestThreadSafety sample and used it to fix this issue. If there is no further issue, I will release Nuget package with version 1.4.5 on Monday.

The issue with WpfSvgRendering: It was calling the base.AfterRender too early. Calling the base.AfterRender sets the IsReady property to true, making it available and rightly taken by other threads.

The issue with SvgList: The use of static dictionary. I have removed the static keyword for now, and will look into a better data structure later.

[atmgrifter00]

We don't know of anything else that would hold up a new release from our (me and @astarche) perspective.

[paulushub]

@atmgrifter00 Thanks for the feedback. The Nuget package is now publicly available and tagged as Version 1.5.0. I had already started tagging the sources as version 1.5.0 so decided to keep that version so as not to break the W3C test logs.
Thanks for the help in tracing and fixing this issue.

[atmgrifter00]

Well, crud. I've found another issue, but as it's unrelated to the other one, I'm just going to have to file a new one.

[paulushub]

So far it seems the fix to this issue is holding!