1679911 - HttpChannelChild::TrySendDeletingChannel() doesn't work when called off the main thread

Status: RESOLVED FIXED

(Core :: Networking: HTTP, task, P2)

Description

[Andrew McCreight [:mccr8]]

This is another issue I noticed while reading over HttpChannelChild::Release. I'm not sure if it matters in practice.

Simplified a bit, the method does this:

  if (!mDeletingChannelSent.compareExchange(false, true)) {
    return;
  }

  if (NS_IsMainThread()) {
    ...
    return;
  }

  ...

  DebugOnly<nsresult> rv = neckoTarget->Dispatch(
      NewNonOwningRunnableMethod(
          "net::HttpChannelChild::TrySendDeletingChannel", this,
          &HttpChannelChild::TrySendDeletingChannel),
      NS_DISPATCH_NORMAL);

When called off the main thread when mDeletingChannelSent is false, it will first set mDeletingChannelSent to true. Then it will dispatch a runnable to the main thread that retries the method. Except that when it runs it will immediately return because we set mDeletingChannelSent to true before dispatching the runnable.

It feels like two possible solutions would be to either set mDeletingChannelSent back to false before dispatching the runnable (maybe this is a bad idea because the runnable has a non-owning call to |this|?) or calling a variant of TrySendDeletingChannel that skips the mDeletingChannelSent check (and presumably the main thread check). It is also possible that this is never (or rarely?) called off the main thread so this stuff could be deleted or turned into a crash or something.

Comment 1

[Kershaw Chang [:kershaw]]

HttpChannelChild::TrySendDeletingChannel should be only called on main thread, so we should add an assertion to force this.

Comment 2

[Sunil Mayya]

Attachment: Bug 1679911 - enforce HttpChannelChild::TrySendDeletingChannel() is called only in main thread. r=#necko

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1feadb20c735
enforce HttpChannelChild::TrySendDeletingChannel() is called only in main thread. r=necko-reviewers,kershaw

Comment 4

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/1feadb20c735