New issue
Advanced search Search tips

Issue 879307 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Audit and fix leaks of Mach host and thread ports

Project Member Reported by a...@chromium.org, Aug 30

Issue description

Description: Show this description
The one in webrtc looks suspicous to me because it's in GetThreadCpuTimeNanos(), which looks like it may be called every frame(?!).
That can't possibly be happening, otherwise we'd be blowing up our port usage, right?
Components: Internals>Core
Summary: Audit and fix leaks of Mach host and thread ports (was: Mach leaks)
I just noticed that the swiftshader Host.cpp one is guarded by a defined(__ppc__) - so that's okay (I've already got a CL out to fix it in LLVM though).
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07d0a85ba6fffb6c527c3473ad2474274327e062

commit 07d0a85ba6fffb6c527c3473ad2474274327e062
Author: Robert Sesek <rsesek@chromium.org>
Date: Fri Aug 31 14:24:32 2018

Use ScopedMachSendRight in media::NumberOfPhysicalProcessors().

This is not leaking, but it is easier to audit for leaks when using
the scoper.

Bug:  879307 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I3d0fa36922feb1a4c1ffef293b97d030c8793e14
Reviewed-on: https://chromium-review.googlesource.com/1196967
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588015}
[modify] https://crrev.com/07d0a85ba6fffb6c527c3473ad2474274327e062/media/audio/mac/audio_low_latency_input_mac.cc

Re 2: The function is called only in the standalone performance tests and isn't included in the chrome. This actually may explain why we sometimes have random failures for mac performance test bots.

Thanks a lot for catching this.
Cc: ilnik@chromium.org
Re comment 8, that makes sense. That function leaks ports so aggressively that it would have surely been caught had it shipped with Chrome. Which means that bug 878071 is caused by other issues...
https://reviews.llvm.org/D51517 for LLVM, which may eventually get rolled into swiftshader.
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 4

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/b0800519b0dce9852ad77176a163cbd16e50a2c1

commit b0800519b0dce9852ad77176a163cbd16e50a2c1
Author: Robert Sesek <rsesek@chromium.org>
Date: Tue Sep 04 13:43:24 2018

Do not leak the Mach thread port in GetThreadCpuTimeNanos().

Bug:  chromium:879307 
Change-Id: Ia6b5b3ea4684354d8a21dc85e43f67166832cc19
Reviewed-on: https://webrtc-review.googlesource.com/96980
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#24554}
[modify] https://crrev.com/b0800519b0dce9852ad77176a163cbd16e50a2c1/rtc_base/cpu_time.cc

Comment 12 Deleted

Comment 13 Deleted

Status: Fixed (was: Assigned)
The above CL in #11 has rolled into Chromium but Bugdroid is down for maintenance so it hasn't posted.

The LLVM CL hasn't been downstreamed into Switfshader yet, but since it's PPC-only, I don't see an urgent need to do so.
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bce785d4f1bbb8a681499b39accdec5f94e8403

commit 5bce785d4f1bbb8a681499b39accdec5f94e8403
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Sep 04 18:25:27 2018

Roll src/third_party/webrtc 46d65e2c2cc0..b0800519b0dc (1 commits)

https://webrtc.googlesource.com/src.git/+log/46d65e2c2cc0..b0800519b0dc


git log 46d65e2c2cc0..b0800519b0dc --date=short --no-merges --format='%ad %ae %s'
2018-09-04 rsesek@chromium.org Do not leak the Mach thread port in GetThreadCpuTimeNanos().


Created with:
  gclient setdep -r src/third_party/webrtc@b0800519b0dc

The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG= chromium:879307 
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: If9e374fbae31d1b49bf47a89c41e5815ca475afb
Reviewed-on: https://chromium-review.googlesource.com/1204610
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#588590}
[modify] https://crrev.com/5bce785d4f1bbb8a681499b39accdec5f94e8403/DEPS

Sign in to add a comment