New issue
Advanced search Search tips

Issue 877511 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Aug 29
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocked on:
issue 812334


Sign in to add a comment

Some Cronet native tests fail ASAN and TSAN

Project Member Reported by pauljensen@chromium.org, Aug 24 
I CQ'd a CL (https://chromium-review.googlesource.com/c/chromium/src/+/1178666) and it failed ASAN and TSAN bots.  After staring at it for a while I suspected it wasn't my CL.

I tried a more-or-less empty CL and it also failed ASAN and TSAN bots:
https://chromium-review.googlesource.com/c/chromium/src/+/1188610

The empty CL failed ASAN and TSAN on just the UrlRequestTest.PerfTest test but if you look at the logging I added you can see it should also be failing ASAN on UrlRequestTest.TestCancel as well (like my original CL did).  Perhaps my original CL failed TestCancel because it simplified the threading of the test which somehow facilitated ASAN.

Anyhow we need to fix these tests because otherwise we cannot land and Cronet CLs because they get stuck in the CQ.

Comment 1 by pauljensen@chromium.org, Aug 24 

So UrlRequestTest.TestCancel has nine test cases:
#1-#5 don't alloc a buffer
#6 does and it appears to leak
#7-#8 does but frees it in TestUrlRequestCallback::OnReadCompleted()
#9 does but ~IOBufferWithCronet_Buffer() frees it

This may be related to  Issue 812334 .  There's already a comment in case #6 about that bug.

The leak in #6 happens when Cancel() is posted right before OnResponseStarted() calls Read().
The comment for  Issue 812334  mentions "'OnReadCompleted' callback may arrive AFTER 'OnCanceled'" and if the executor is shutdown I imagine the buffer is getting leaked.

CronetURLRequest::NetworkTasks::OnReadCompleted() transfers buffer ownership (though it doesn't clear its refptr until afterwards...weird) to Cronet_UrlRequestImpl::NetworkTasks::OnReadCompleted() which passes ownership into the closure that's posted to the executor...which I imagine may leak if it never gets run and isn't properly freed.

Comment 2 by pauljensen@chromium.org, Aug 24

Blockedon: 812334

Comment 4 by mef@chromium.org, Aug 27 

Hrm, the failures more-or-less empty CL and it also failed ASAN and TSAN bots:
https://chromium-review.googlesource.com/c/chromium/src/+/1188610 seems to point to 'Excessive output' in PerfTest.

Comment 5 by pauljensen@chromium.org, Aug 29

Status: Fixed (was: Available)
mef@ is right, my test CL was failing for unrelated reasons.  Anyhow I'm closing this as I think I addressed the issues I was seeing by fixing other bugs in r587167.
►

Sign in to add a comment