Bug 1754004 - Part 15: Don't track mOriginalInput in nsPipe, r=asuth

As discovered by TSan, this member could be raced on in some edge-cases if
cloned pipe streams raced on multiple threads to be destroyed. This change
instead moves the `nsIPipe` implementation to be on a seperate type from
`nsPipe`, allowing `mOriginalInput` to no longer need to be tracked.

This technically has slightly different behaviour than the previous
implementation for JS code, as code keeping the `nsIPipe` alive will now also
be keeping the output stream reference alive directly, rather than only holding
the `nsIPipe`, meaning that `pipe.outputStream` can be read multiple times
independently without implicitly closing the underlying stream. Based on a
quick read of the few remaining uses of `nsIPipe` in JS code, I don't think
this will be an issue, and it may actually be more intuitive and consistent.

Depends on D142127

Differential Revision: https://phabricator.services.mozilla.com/D145362
This commit is contained in:
Nika Layzell 2022-05-03 23:30:38 +00:00
Родитель 16d8f34b47
Коммит d5a1b01ff8
1 изменённых файлов: 73 добавлений и 105 удалений

Просмотреть файл

@ -269,19 +269,20 @@ class nsPipeOutputStream : public nsIAsyncOutputStream, public nsIClassInfo {
//-----------------------------------------------------------------------------
class nsPipe final : public nsIPipe {
class nsPipe final {
public:
friend class nsPipeInputStream;
friend class nsPipeOutputStream;
friend class AutoReadSegment;
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSIPIPE
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsPipe)
// nsPipe methods:
nsPipe();
// public constructor
friend nsresult NS_NewPipe2(nsIAsyncInputStream**, nsIAsyncOutputStream**,
bool, bool, uint32_t, uint32_t);
private:
nsPipe(uint32_t aSegmentSize, uint32_t aSegmentCount);
~nsPipe();
//
@ -336,12 +337,6 @@ class nsPipe final : public nsIPipe {
// Only usable while mReentrantMonitor is locked.
nsTArray<nsPipeInputStream*> mInputList;
// But hold a strong ref to our original input stream. For backward
// compatibility we need to be able to consistently return this same
// object from GetInputStream(). Note, mOriginalInput is also stored
// in mInputList as a weak ref.
RefPtr<nsPipeInputStream> mOriginalInput;
ReentrantMonitor mReentrantMonitor MOZ_UNANNOTATED;
nsSegmentedBuffer mBuffer;
@ -356,7 +351,6 @@ class nsPipe final : public nsIPipe {
// |mStatus| is protected by |mReentrantMonitor|.
nsresult mStatus;
bool mInited;
};
//-----------------------------------------------------------------------------
@ -483,96 +477,24 @@ class MOZ_STACK_CLASS AutoReadSegment final {
// nsPipe methods:
//-----------------------------------------------------------------------------
nsPipe::nsPipe()
nsPipe::nsPipe(uint32_t aSegmentSize, uint32_t aSegmentCount)
: mOutput(this),
mOriginalInput(new nsPipeInputStream(this)),
mReentrantMonitor("nsPipe.mReentrantMonitor"),
mMaxAdvanceBufferSegmentCount(0),
// protect against overflow
mMaxAdvanceBufferSegmentCount(
std::min(aSegmentCount, UINT32_MAX / aSegmentSize)),
mWriteSegment(-1),
mWriteCursor(nullptr),
mWriteLimit(nullptr),
mStatus(NS_OK),
mInited(false) {
mInputList.AppendElement(mOriginalInput);
}
nsPipe::~nsPipe() = default;
NS_IMPL_ADDREF(nsPipe)
NS_IMPL_QUERY_INTERFACE(nsPipe, nsIPipe)
NS_IMETHODIMP_(MozExternalRefCountType)
nsPipe::Release() {
MOZ_DIAGNOSTIC_ASSERT(int32_t(mRefCnt) > 0, "dup release");
nsrefcnt count = --mRefCnt;
NS_LOG_RELEASE(this, count, "nsPipe");
if (count == 0) {
delete (this);
return 0;
}
// Avoid racing on |mOriginalInput| by only looking at it when
// the refcount is 1, that is, we are the only pointer (hence only
// thread) to access it.
if (count == 1 && mOriginalInput) {
mOriginalInput = nullptr;
return 1;
}
return count;
}
NS_IMETHODIMP
nsPipe::Init(bool aNonBlockingIn, bool aNonBlockingOut, uint32_t aSegmentSize,
uint32_t aSegmentCount) {
mInited = true;
if (aSegmentSize == 0) {
aSegmentSize = DEFAULT_SEGMENT_SIZE;
}
if (aSegmentCount == 0) {
aSegmentCount = DEFAULT_SEGMENT_COUNT;
}
// protect against overflow
uint32_t maxCount = uint32_t(-1) / aSegmentSize;
if (aSegmentCount > maxCount) {
aSegmentCount = maxCount;
}
mStatus(NS_OK) {
// The internal buffer is always "infinite" so that we can allow
// the size to expand when cloned streams are read at different
// rates. We enforce a limit on how much data can be buffered
// ahead of the fastest reader in GetWriteSegment().
nsresult rv = mBuffer.Init(aSegmentSize, UINT32_MAX);
if (NS_FAILED(rv)) {
return rv;
}
mMaxAdvanceBufferSegmentCount = aSegmentCount;
mOutput.SetNonBlocking(aNonBlockingOut);
mOriginalInput->SetNonBlocking(aNonBlockingIn);
return NS_OK;
MOZ_ALWAYS_SUCCEEDS(mBuffer.Init(aSegmentSize, UINT32_MAX));
}
NS_IMETHODIMP
nsPipe::GetInputStream(nsIAsyncInputStream** aInputStream) {
if (NS_WARN_IF(!mInited)) {
return NS_ERROR_NOT_INITIALIZED;
}
RefPtr<nsPipeInputStream> ref = mOriginalInput;
ref.forget(aInputStream);
return NS_OK;
}
NS_IMETHODIMP
nsPipe::GetOutputStream(nsIAsyncOutputStream** aOutputStream) {
if (NS_WARN_IF(!mInited)) {
return NS_ERROR_NOT_INITIALIZED;
}
NS_ADDREF(*aOutputStream = &mOutput);
return NS_OK;
}
nsPipe::~nsPipe() = default;
void nsPipe::PeekSegment(const nsPipeReadState& aReadState, uint32_t aIndex,
char*& aCursor, char*& aLimit) {
@ -1816,29 +1738,75 @@ nsresult NS_NewPipe2(nsIAsyncInputStream** aPipeIn,
nsIAsyncOutputStream** aPipeOut, bool aNonBlockingInput,
bool aNonBlockingOutput, uint32_t aSegmentSize,
uint32_t aSegmentCount) {
nsPipe* pipe = new nsPipe();
nsresult rv = pipe->Init(aNonBlockingInput, aNonBlockingOutput, aSegmentSize,
aSegmentCount);
if (NS_FAILED(rv)) {
NS_ADDREF(pipe);
NS_RELEASE(pipe);
return rv;
}
RefPtr<nsPipe> pipe =
new nsPipe(aSegmentSize ? aSegmentSize : DEFAULT_SEGMENT_SIZE,
aSegmentCount ? aSegmentCount : DEFAULT_SEGMENT_COUNT);
// These always succeed because the pipe is initialized above.
MOZ_ALWAYS_SUCCEEDS(pipe->GetInputStream(aPipeIn));
MOZ_ALWAYS_SUCCEEDS(pipe->GetOutputStream(aPipeOut));
RefPtr<nsPipeInputStream> pipeIn = new nsPipeInputStream(pipe);
pipe->mInputList.AppendElement(pipeIn);
RefPtr<nsPipeOutputStream> pipeOut = &pipe->mOutput;
pipeIn->SetNonBlocking(aNonBlockingInput);
pipeOut->SetNonBlocking(aNonBlockingOutput);
pipeIn.forget(aPipeIn);
pipeOut.forget(aPipeOut);
return NS_OK;
}
////////////////////////////////////////////////////////////////////////////////
// Thin nsIPipe implementation for consumers of the component manager interface
// for creating pipes. Acts as a thin wrapper around NS_NewPipe2 for JS callers.
class nsPipeHolder final : public nsIPipe {
public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSIPIPE
private:
~nsPipeHolder() = default;
nsCOMPtr<nsIAsyncInputStream> mInput;
nsCOMPtr<nsIAsyncOutputStream> mOutput;
};
NS_IMPL_ISUPPORTS(nsPipeHolder, nsIPipe)
NS_IMETHODIMP
nsPipeHolder::Init(bool aNonBlockingInput, bool aNonBlockingOutput,
uint32_t aSegmentSize, uint32_t aSegmentCount) {
if (mInput || mOutput) {
return NS_ERROR_ALREADY_INITIALIZED;
}
return NS_NewPipe2(getter_AddRefs(mInput), getter_AddRefs(mOutput),
aNonBlockingInput, aNonBlockingOutput, aSegmentSize,
aSegmentCount);
}
NS_IMETHODIMP
nsPipeHolder::GetInputStream(nsIAsyncInputStream** aInputStream) {
if (mInput) {
*aInputStream = do_AddRef(mInput).take();
return NS_OK;
}
return NS_ERROR_NOT_INITIALIZED;
}
NS_IMETHODIMP
nsPipeHolder::GetOutputStream(nsIAsyncOutputStream** aOutputStream) {
if (mOutput) {
*aOutputStream = do_AddRef(mOutput).take();
return NS_OK;
}
return NS_ERROR_NOT_INITIALIZED;
}
nsresult nsPipeConstructor(nsISupports* aOuter, REFNSIID aIID, void** aResult) {
if (aOuter) {
return NS_ERROR_NO_AGGREGATION;
}
nsPipe* pipe = new nsPipe();
NS_ADDREF(pipe);
RefPtr<nsPipeHolder> pipe = new nsPipeHolder();
nsresult rv = pipe->QueryInterface(aIID, aResult);
NS_RELEASE(pipe);
return rv;
}