1359847 - Better handle case when TCP FastOpen is disabled on linux and in some cases on Windows10

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement)

Description

[Dragana Damjanovic [:dragana]]

This is hard to trigger, but theoretically can be triggered. And it is more of a problem for h2 than h1.

It is triggered when TFO is disabled on linux or in the older versions of Win10 that do not support TFO. The only way to figure out if TFO is disabled(linux) or not present (Win10 that do not support TFO) is to call sendto. So we start using fast open and open a nsHttpConnections with TFO. That first connection will use sendto which will fail. And current implementation will get in a strange state - nsHttpConnection is a TFO connection(socket is not connected) but the fallback mechanism is destroyed.

if that connection succeeds everything is fine and also if it fails and the corresponding transaction does not need to be restarted, but I am afraid that there will be a problem on h2 if connection fails and needs to be restated.

I made such a connection to behave as if TFO is still on, so that our TFO fallback mechanisms will handle it correctly.

Comment 1

[Dragana Damjanovic [:dragana]]

Attachment: bug_tfo_not_supported_restart.patch

Comment 2

[Patrick McManus [:mcmanus]]

I'm going to guess that this patch is actually the cause of https://bugzilla.mozilla.org/show_bug.cgi?id=1359938#c3. we can always vindicate it with a green try run :)

The failed assert from nsNSSIOLayer

static void
reportHandshakeResult(int32_t bytesTransferred, bool wasReading, PRErrorCode err)
{
  uint32_t bucket;

  // A negative bytesTransferred or a 0 read are errors.
  if (bytesTransferred > 0) {
    bucket = 0;
  } else if ((bytesTransferred == 0) && !wasReading) {
    // PR_Write() is defined to never return 0, but let's make sure.
    // https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSPR/Reference/PR_Write.
    MOZ_ASSERT(false);
    bucket = 671;


2017-04-26T18:24:51.672745Z] 18:24:51     INFO - PID 1960 | #01: (anonymous namespace)::checkHandshake(int, bool, PRFileDesc*, nsNSSSocketInfo*) (/home/worker/workspace/build/application/firefox/libxul.so)
[task 2017-04-26T18:24:51.677826Z] 18:24:51     INFO - PID 1960 | #02: PSMSend(PRFileDesc*, void const*, int, int, unsigned int) (/home/worker/workspace/build/application/firefox/libxul.so)
[task 2017-04-26T18:24:51.682770Z] 18:24:51     INFO - PID 1960 | #03: nsSSLIOLayerWrite(PRFileDesc*, void const*, int) (/home/worker/workspace/build/application/firefox/libxul.so)
[task 2017-04-26T18:24:51.715338Z] 18:24:51     INFO - PID 1960 | #04: PR_Write (/home/worker/workspace/build/application/firefox/libnspr4.so)
[task 2017-04-26T18:24:51.721116Z] 18:24:51     INFO - PID 1960 | #05: mozilla::net::nsSocketOutputStream::Write(char const*, unsigned int, unsigned int*) (/home/worker/workspace/build/application/firefox/libxul.so)
[task 2017-04-26T18:24:51.726144Z] 18:24:51     INFO - PID 1960 | #06: mozilla::net::nsSocketTransport::OnSocketReady(PRFileDesc*, short) (/home/worker/workspace/build/application/firefox/libxul.so)
[task 2017-04-26T18:24:51.731097Z] 18:24:51     INFO - PID 1960 | #07: mozilla::net::nsSocketTransportService::DoPollIteration(mozilla::BaseTimeDuration<mozilla::TimeDurationValueCalculator>*) (/home/worker/workspace/build/application/firefox/libxul.so)
[task 2017-04-26T18:24:51.736306Z] 18:24:51     INFO - PID 1960 | #08: mozilla::net::nsSocketTransportService::Run() (/home/worker/workspace/build/application/firefox/libxul.so)



is this maybe a socket event that has been queued but not run before the dtor logic that goes in this patch?

Comment 3

[Dragana Damjanovic [:dragana]]

It is not this one it is the patch from 1359938.

In 1359938, I call mOutput.Write(null, 0, ...) to force TCPFastOpenLayer to drain its buffer. That returns 0: TCPFastOpenLayer calls "return fd->lower->methods->send(buf,amount ...)", amount is 0 so that returns 0
and nss does not like it :)

To fix this I need to check if amount of data to write is 0 and not call fd->lower->methods->send. I will test it in the morning.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e44d09824d9e
Improve transaction restart if tfo is not supported. r=mcmanus

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e44d09824d9e