Creating HtmlParser for each HtmlSanitizer.Sanitize call?

[DmitryZhuravkin]

Hey everyone,
I used HtmlSanitizer (version 5.0.353) in multi threaded app with a big amount of strings need to be sanitized before storing into DB. I use just one instance of HtmlSanitizer across all threads. The instance configuration is pretty simple:

_plainSanitizer = new HtmlSanitizer { KeepChildNodes = true };
_plainSanitizer.AllowDataAttributes = false;
_plainSanitizer.AllowedAtRules.Clear();
_plainSanitizer.AllowedAttributes.Clear();
_plainSanitizer.AllowedCssProperties.Clear();
_plainSanitizer.AllowedSchemes.Clear();
_plainSanitizer.AllowedTags.Clear();
_plainSanitizer.UriAttributes.Clear();

Usage:
public static string SanitizeAll(this string source) { var result = _plainSanitizer.Sanitize(source); return result; }
But after profiling I`ve noted that for each Sanitize call a new instance of HtmlParser is created, and this degradates performance significantly. My plan is to use just one instance of HtmlParser, something like this (performance looks pretty good for this scenario):

private static readonly HtmlSanitizer _sanitizer;
private static readonly HtmlParser _parser = new HtmlParser();

static SanitizeHelper()
{
    _sanitizer = new HtmlSanitizer
    {
        KeepChildNodes = true,
        AllowDataAttributes = false,
        HtmlParserFactory = () => _parser
    };

    _sanitizer.AllowedAtRules.Clear();
    _sanitizer.AllowedAttributes.Clear();
    _sanitizer.AllowedCssProperties.Clear();
    _sanitizer.AllowedSchemes.Clear();
    _sanitizer.AllowedTags.Clear();
    _sanitizer.UriAttributes.Clear();
}

But I am not sure this is safe in terms of multi threading. Potentially, the approach to use one instance of HtmlParser per call is a planned decision. Could you please give me few notes about this decision? What do you think, will the approach with single instance work better in this case?

Thanks.

[mganss]

It should be ok. There is no specific motivation regarding thread safety behind the decision to use a fresh instance of HtmlParser for each call to Sanitize(). OTOH AngleSharp seems to make no statements regarding thread safety.

I've ran a modified version of this test using a static instance of HtmlParser and it works fine:

HtmlSanitizer/test/HtmlSanitizer.Tests/Tests.cs

Lines 2883 to 2927 in 4a46498

	[Fact]
	public void ThreadTest()
	{
	const int numThreads = 16;
	const int numRuns = 1000;
	var random = new Random(615322944);
	for (int i = 0; i < numRuns; i++)
	{
	var allGo = new ManualResetEvent(false);
	Exception firstException = null;
	var failures = 0;
	var fixture = new HtmlSanitizerFixture();
	var tests = new HtmlSanitizerTests(fixture);
	var waiting = numThreads;
	var methods = typeof(HtmlSanitizerTests).GetTypeInfo().GetMethods()
	.Where(m => m.GetCustomAttributes(typeof(Xunit.FactAttribute), false).Any())
	.Where(m => m.Name != "ThreadTest");
	var threads = Shuffle(methods, random)
	.Take(numThreads)
	.Select(m => new Thread(() =>
	{
	try
	{
	if (Interlocked.Decrement(ref waiting) == 0) allGo.Set();
	m.Invoke(tests, null);
	}
	#pragma warning disable CA1031 // Do not catch general exception types
	catch (Exception ex)
	{
	Interlocked.CompareExchange(ref firstException, ex, null);
	Interlocked.Increment(ref failures);
	}
	#pragma warning restore CA1031 // Do not catch general exception types
	})).ToList();
	foreach (var thread in threads)
	thread.Start();
	foreach (var thread in threads)
	thread.Join();
	Assert.Null(firstException);
	Assert.Equal(0, failures);
	}
	}

[DmitryZhuravkin]

Thanks a lot!