Thread Safety Warning in XHR String

[tmbrbr]

There is a compiler warning about thread safety in the XHR String implementation.

01:18:37  41:24.10 In file included from Unified_cpp_dom_xhr0.cpp:29:
01:18:37  41:24.11 /home/jenkins/agent/workspace/foxhound/dom/xhr/XMLHttpRequestString.cpp:50:5: warning: reading variable 'mData' requires holding mutex 'mMutex' [-Wthread-safety-analysis]
01:18:37  41:24.11    50 |     mData.Taint().concat(aTaint, aIndex);
01:18:37  41:24.11       |     ^

[leeN]

Both the mentioned, as well as a warning in the HTMLParser, seem to rely on external locks.

• Warning: dom/xhr/XMLHttpRequestString.cpp:50:5 [-Wthread-safety-analysis] reading variable 'mData' requires holding mutex 'mMutex'
• Warning: parser/html/nsHtml5StreamParser.cpp:1734:11 [-Wthread-safety-analysis] calling function 'DoDataAvailable' requires holding mutex 'parser->mTokenizerMutex' exclusively

For the first warning, it is directly stated in a comment; for the second, the comment refers to the function the taint-aware version was copied from.

// Called under lock by function ptr
/* static */
nsresult nsHtml5StreamParser::CopySegmentsToParserNoTaint(
    nsIInputStream* aInStream, void* aClosure, const char* aFromSegment,
    uint32_t aToOffset, uint32_t aCount,
    uint32_t* aWriteCount) MOZ_NO_THREAD_SAFETY_ANALYSIS {
  nsHtml5StreamParser* parser = static_cast<nsHtml5StreamParser*>(aClosure);

  parser->DoDataAvailable(AsBytes(Span(aFromSegment, aCount)), EmptyTaint);
  // Assume DoDataAvailable consumed all available bytes.
  *aWriteCount = aCount;
  return NS_OK;
}

This is the original, and the following is warned about:

/* static */ nsresult
nsHtml5StreamParser::CopySegmentsToParser(
  nsITaintawareInputStream *aInStream, void *aClosure, const char *aFromSegment,
  uint32_t aToOffset, uint32_t aCount, const StringTaint& aTaint, uint32_t *aWriteCount) {
  nsHtml5StreamParser* parser = static_cast<nsHtml5StreamParser*>(aClosure);

  parser->DoDataAvailable(AsBytes(Span(aFromSegment, aCount)), aTaint); // <- warning here
  // Assume DoDataAvailable consumed all available bytes.
  *aWriteCount = aCount;
  return NS_OK;
}

So, I'd conclude it is sufficient to add the MOZ_NO_THREAD_SAFETY_ANALYSIS attribute to both functions. I'll open a PR.

[tmbrbr]

Thanks for looking into this!

[tmbrbr]

Closed by #197