Closed Bug 1627572 Opened 5 years ago Closed 5 years ago

Intermittent tsan: ThreadSanitizer: data race /builds/worker/checkouts/gecko/xpcom/io/FilePreferences.cpp:272:7 in mozilla::FilePreferences::IsAllowedPath(nsTSubstring<char> const&)

Categories

(Core :: XPCOM, defect, P5)

Product:
Core
Component:
XPCOM
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox77 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: mayhemer)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Detailed Crash Information
12.01 KB, text/plain
Details
Bug 1627572 - Use an atomic bool for a quick-return check in FilePreferences::IsAllowedPath, r=michal
47 bytes, text/x-phabricator-request
Details | Review

Filed by: ncsoregi [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=296356051&repo=autoland
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Ti979cL-S9i4yZGbre7MQg/runs/0/artifacts/public/logs/live_backing.log

[task 2020-04-05T23:05:44.396Z] 23:05:44 INFO - GECKO(1284) | #54 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:4761:8 (libxul.so+0x633ce23)
[task 2020-04-05T23:05:44.397Z] 23:05:44 INFO - GECKO(1284) | #55 XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:4812:21 (libxul.so+0x633d174)
[task 2020-04-05T23:05:44.397Z] 23:05:44 INFO - GECKO(1284) | #56 mozilla::BootstrapImpl::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/Bootstrap.cpp:45:12 (libxul.so+0x6347d22)
[task 2020-04-05T23:05:44.397Z] 23:05:44 INFO - GECKO(1284) | #57 do_main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:217:22 (firefox+0xc8b63)
[task 2020-04-05T23:05:44.397Z] 23:05:44 INFO - GECKO(1284) | #58 main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:331:16 (firefox+0xc8b63)
[task 2020-04-05T23:05:44.398Z] 23:05:44 INFO - GECKO(1284) | SUMMARY: ThreadSanitizer: data race /builds/worker/checkouts/gecko/xpcom/io/FilePreferences.cpp:272:7 in mozilla::FilePreferences::IsAllowedPath(nsTSubstring<char> const&)
[task 2020-04-05T23:05:44.398Z] 23:05:44 INFO - GECKO(1284) | ==================
[task 2020-04-05T23:08:12.872Z] 23:08:12 INFO - runtests.py | Waiting for browser...
[task 2020-04-05T23:08:12.872Z] 23:08:12 INFO - TEST-INFO | Main app process: killed by SIGIOT
[task 2020-04-05T23:08:12.873Z] 23:08:12 INFO - Buffered messages finished
[task 2020-04-05T23:08:12.873Z] 23:08:12 ERROR - TEST-UNEXPECTED-FAIL | automation.py | application terminated with exit code -6
[task 2020-04-05T23:08:12.873Z] 23:08:12 INFO - runtests.py | Application ran for: 0:03:00.053382
[task 2020-04-05T23:08:12.874Z] 23:08:12 INFO - zombiecheck | Reading PID log: /tmp/tmp4C5rM3pidlog
[task 2020-04-05T23:08:12.874Z] 23:08:12 INFO - ==> process 1284 launched child process 1299
[task 2020-04-05T23:08:12.874Z] 23:08:12 INFO - zombiecheck | Checking for orphan process with PID: 1299
[task 2020-04-05T23:08:12.874Z] 23:08:12 INFO - Traceback (most recent call last):
[task 2020-04-05T23:08:12.875Z] 23:08:12 INFO - File "/builds/worker/workspace/build/tests/mochitest/runtests.py", line 2871, in doTests
[task 2020-04-05T23:08:12.875Z] 23:08:12 INFO - e10s=options.e10s
[task 2020-04-05T23:08:12.875Z] 23:08:12 INFO - File "/builds/worker/workspace/build/tests/mochitest/runtests.py", line 2388, in runApp
[task 2020-04-05T23:08:12.876Z] 23:08:12 INFO - raise exc(value).with_traceback(tb)
[task 2020-04-05T23:08:12.876Z] 23:08:12 INFO - AttributeError: 'timeout' object has no attribute 'with_traceback'
[task 2020-04-05T23:08:12.876Z] 23:08:12 ERROR - Automation Error: Received unexpected exception while running application
[task 2020-04-05T23:08:12.876Z] 23:08:12 ERROR -

Blocks: tsan
This is an automated crash issue comment: Summary: ThreadSanitizer: data race [@ mozilla::FilePreferences::IsAllowedPath] vs. [@ pthread_create] Build version: autoland revision 465fa77baabe4a7e6e7c4f3a75abe9659c920ed6 For detailed crash information, see attachment. The second stack is missing, that indicates that it is likely suppressed and explains why this shows up only intermittently. We should still be able to solve this because it appears to be a simple problem: The global `sBlacklistEmpty` needs to be made an atomic.

NI based on comment 2 and hg annotate.

Flags: needinfo?(honzab.moz)

I will remove this quick check out of the lock for TSan builds. This is a hot path. Initially the value is at false. Later highly likely the value will flip to true in vanila Firefox. Unless access to a static global var can be seen by other threads somehow falsely as true before it actually is set to it, then I don't see a reason to use an atomic. Knowledge of processor caches is not good enough for me, but common sense tells me that should never happen.

Flags: needinfo?(honzab.moz) → needinfo?(choller)

I think the risk is rather that the update to true is not seen by other threads and extra locks are being taken and/or that the compiler makes weird optimizations assuming there is no race.

I don't think using a TSan only solution is the right approach here, instead using Atomic<bool, Relaxed> is probably the right approach with negligible overhead. I would only go for the ifdef solution if you can really measure the overhead coming from a relaxed Atomic (in most cases, there is none).

Flags: needinfo?(choller)
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Pushed by honzab.moz@firemni.cz: https://hg.mozilla.org/integration/autoland/rev/954957939e71 Use an atomic bool for a quick-return check in FilePreferences::IsAllowedPath, r=michal

https://hg.mozilla.org/mozilla-central/rev/954957939e71

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox77: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: