Bugzilla

So, as I said, we clearly have a possibility of race between sending a message from the parent process to set a filter (new listener) on the child http channel and cancellation of the child channel.  I captured [a log](https://mayhemer.github.io/backtrack-analyzer/?https://raw.githubusercontent.com/mayhemer/data/master/bug1403546-set-new-listener-on-child.zip#) - please read as a stack trace (backwards) - with an added objective marker to `HttpBaseChannel::SetNewListener` showing that we:

- [Set a new listener](https://mayhemer.github.io/backtrack-analyzer/?https://raw.githubusercontent.com/mayhemer/data/master/bug1403546-set-new-listener-on-child.zip#ref-28) on the parent process
- this happens from inside [a code](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/toolkit/modules/addons/WebRequest.jsm#604) handling `http-on-modify-request`
- that code also sends - through idle-dispatch (!) - a message to the webextension process that then sends InitStreamFilter message back to the parent process that ends up in [nsHttpChannel::AttachStreamFilter](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/netwerk/protocol/http/nsHttpChannel.cpp#6877) which is sending a message AttachStreamFilter to the http child channel.

The http child channel can be in whatever state.  If it has been cancelled (page load cancel, reload, navigation), it may have already notified OnStopRequest and release its listener.  The cancellation messages is pending somewhere on the way to the parent process, but it doesn't matter as checking cancellation state in nsHttpChannel::AttachStreamFilter would still be racy.

So, the fix here is to check if the child channel is not already canceled.  It's valid to fail (bypass) setting the filter then, as the whole channel pair will go away soon anyway.  No idea how we should inform the extension about this - I mean if at all to inform it somehow the filter can't be set and will not fire.

So, as I said, we clearly have a possibility of race between sending a message from the parent process to set a filter (new listener) on the child http channel and cancellation of the child channel.  I captured [a log](https://mayhemer.github.io/backtrack-analyzer/?https://raw.githubusercontent.com/mayhemer/data/master/bug1403546-set-new-listener-on-child.zip#) - please read as a stack trace (backwards) - with an added objective marker to `HttpBaseChannel::SetNewListener` showing that we:

- Set a new listener (search for `http-on-modify-request`) on the parent process
- this happens from inside [a code](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/toolkit/modules/addons/WebRequest.jsm#604) handling `http-on-modify-request`
- that code also sends - through idle-dispatch (!) - a message to the webextension process that then sends InitStreamFilter message back to the parent process that ends up in [nsHttpChannel::AttachStreamFilter](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/netwerk/protocol/http/nsHttpChannel.cpp#6877) which is sending a message AttachStreamFilter to the http child channel.

The http child channel can be in whatever state.  If it has been cancelled (page load cancel, reload, navigation), it may have already notified OnStopRequest and release its listener.  The cancellation messages is pending somewhere on the way to the parent process, but it doesn't matter as checking cancellation state in nsHttpChannel::AttachStreamFilter would still be racy.

So, the fix here is to check if the child channel is not already canceled.  It's valid to fail (bypass) setting the filter then, as the whole channel pair will go away soon anyway.  No idea how we should inform the extension about this - I mean if at all to inform it somehow the filter can't be set and will not fire.

So, as I said, we clearly have a possibility of race between sending a message from the parent process to set a filter (new listener) on the child http channel and cancellation of the child channel.  I captured [a log](https://mayhemer.github.io/backtrack-analyzer/?https://raw.githubusercontent.com/mayhemer/data/master/bug1403546-set-new-listener-on-child.zip#) - please read as a stack trace (backwards) - with an added objective marker to `HttpBaseChannel::SetNewListener` showing that we:

- Set a new listener (search for seconds instance of `SetNewListener`) on the parent process
- this happens from inside [a code](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/toolkit/modules/addons/WebRequest.jsm#604) handling `http-on-modify-request`
- that code also sends - through idle-dispatch (!) - a message to the webextension process that then sends InitStreamFilter message back to the parent process that ends up in [nsHttpChannel::AttachStreamFilter](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/netwerk/protocol/http/nsHttpChannel.cpp#6877) which is sending a message AttachStreamFilter to the http child channel.

The http child channel can be in whatever state.  If it has been cancelled (page load cancel, reload, navigation), it may have already notified OnStopRequest and release its listener.  The cancellation messages is pending somewhere on the way to the parent process, but it doesn't matter as checking cancellation state in nsHttpChannel::AttachStreamFilter would still be racy.

So, the fix here is to check if the child channel is not already canceled.  It's valid to fail (bypass) setting the filter then, as the whole channel pair will go away soon anyway.  No idea how we should inform the extension about this - I mean if at all to inform it somehow the filter can't be set and will not fire.

So, as I said, we clearly have a possibility of race between sending a message from the parent process to set a filter (new listener) on the child http channel and cancellation of the child channel.  I captured [a log](https://mayhemer.github.io/backtrack-analyzer/?https://raw.githubusercontent.com/mayhemer/data/master/bug1403546-set-new-listener-on-child.zip#) - please read as a stack trace (backwards) - with an added objective marker to `HttpBaseChannel::SetNewListener` showing that we:

- Set a new listener (search for seconds instance of `SetNewListener`) on the parent process in nsHttpChannel
- this happens from inside [a code](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/toolkit/modules/addons/WebRequest.jsm#604) handling `http-on-modify-request`
- this code also sends - through idle-dispatch - a message to the webextension process that then sends InitStreamFilter message back to the parent process that ends up in [nsHttpChannel::AttachStreamFilter](https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/netwerk/protocol/http/nsHttpChannel.cpp#6877) which is sending a message AttachStreamFilter to the http child channel.

The http child channel can be in whatever state.  If it has been cancelled (page load cancel, reload, navigation), it may have already notified OnStopRequest and release its listener.  The cancellation messages is pending somewhere on the way to the parent process, but it doesn't matter as checking cancellation state in nsHttpChannel::AttachStreamFilter would still be racy.

So, the fix here is to check if the child channel is not already canceled.  It's valid to fail (bypass) setting the filter then, as the whole channel pair will go away soon anyway.  No idea how we should inform the extension about this - I mean if at all to inform it somehow the filter can't be set and will not fire.