Summary: | REGRESSION (r226485): Many new wasm leaks detected by the leaks bot | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||
Component: | Tools / Tests | Assignee: | Alexey Proskuryakov <ap> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, fpizlo, glenn, jfbastien, jlewis3, joepeck, lforschler, mark.lam, ryanhaddad, sam, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Other | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2018-01-08 13:43:08 PST
Created attachment 331941 [details]
disable poisoning on the bot
Sounds like the short term solution is to disable poisoning on the bot. JF, is this likely to continue working for now?
Comment on attachment 331941 [details] disable poisoning on the bot (In reply to Alexey Proskuryakov from comment #2) > Created attachment 331941 [details] > disable poisoning on the bot > > Sounds like the short term solution is to disable poisoning on the bot. JF, > is this likely to continue working for now? Disabling poisoning should remain a reliable manner to avoid leaks. It looks like you're disabling it only for the leaks bots, which is important since we want to test with poisoning elsewhere. If the leaks program changes in the future, say to ignore the WebKit zones, we should make sure that with JSC_usePoisoning=0 they do detect leaks properly (otherwise we'd just ignore the leaks). Comment on attachment 331941 [details] disable poisoning on the bot Clearing flags on attachment: 331941 Committed r227342: <https://trac.webkit.org/changeset/227342> All reviewed patches have been landed. Closing bug. This did not work, I needed to wrap the additional argument. Created attachment 332169 [details]
do it right
Comment on attachment 332169 [details] do it right View in context: https://bugs.webkit.org/attachment.cgi?id=332169&action=review > Tools/ChangeLog:6 > + https://bugs.webkit.org/show_bug.cgi?id=181400 > + https://bugs.webkit.org/show_bug.cgi?id=181400 > + Double URL Comment on attachment 332169 [details]
do it right
Wait, this still doesn't make sense. This should be built into run-webkit-tests, so that checking for leaks works for engineers.
Created attachment 332176 [details]
do it right
Okay, I hope that it finally makes sense.
Comment on attachment 332176 [details] do it right View in context: https://bugs.webkit.org/attachment.cgi?id=332176&action=review rs=me > Tools/BuildSlaveSupport/build.webkit.org-config/config.json:133 > { "name": "Apple High Sierra (Leaks)", "type": "TestWebKit1Leaks", "builddir": "highsierra-leaks", > "platform": "mac-highsierra", "configuration": "release", "architectures": ["x86_64"], > - "additionalArguments": ["--no-retry-failures", "--no-sample-on-timeout", "--JSC_usePoisoning=0"], > + "additionalArguments": ["--no-retry-failures", "--no-sample-on-timeout"], > "slavenames": ["bot121"] Wondering why I don't see the obvious --leaks and -1 on this "TestWebKit1Leaks" bot. Must be buried somewhere else? > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:433 > + if options.leaks: > + options.additional_env_var.append("JSC_usePoisoning=0") Is leaks intended to work with WebKit2? If so, you should probably also include "__XPC_JSC_usePoisoning=0" as well, so that WebContentProcesses would get it. In practice I think we only run --leaks with -1 right now. > Wondering why I don't see the obvious --leaks and -1 on this "TestWebKit1Leaks" bot. Must be buried somewhere else? Correct, all bots with type TestWebKit1Leaks get that. Maybe "--no-retry-failures", "--no-sample-on-timeout" should be there too, not sure. > Is leaks intended to work with WebKit2? It doesn't work with multi-process in a meaningful way (cf. bug 167851). But I'll add the XPC variant, so that we have fewer things to fix for WebKit2 in the future. Committed http://trac.webkit.org/r227547 |