It looks like this test is failing from too much recursion. Apparently all kinds of random things affect the available stack space to JS but I'm not entirely sure why this only happens on aurora. I think there are ways I can reduce the recursion going on here a lot, but I don't know if it will be upliftable. But then if xpcshell is hitting this it seems likely that real users could hit it too.
Interesting given bug 1158223 that hit recently.
It's not possible to use try to verify a fix here because PGO builds timeout on try. Waiting on a project branch to be reset so I can use it to test a fix I have.
m-a is currently closed because of this bug. What are our options for fixing the bustage in order to reopen the tree? Rather than fix the issue, is there a change that we can backout? If the issue is tests only, are there tests that we should disable? Note that we also currently have Aurora updates disabled and are scheduled to re-enable updates today.
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #14) > m-a is currently closed because of this bug. What are our options for fixing > the bustage in order to reopen the tree? Rather than fix the issue, is there > a change that we can backout? If the issue is tests only, are there tests > that we should disable? I don't know what caused this bustage, it's likely unrelated to the add-ons manager since it seems to be something limiting JS stack space so unless shu has other ideas it could be anything in the aurora merge. I don't know why it is only happening on aurora. It also looks likely to affect users too, unless something about xpcshell means we have less JS stack space for some reason so disabling the test doesn't seem like a good idea. I now have access to a project branch that reproduces the failure and I've pushed a likely fix there to see the results. Assuming it works I should have a patch up for review today.
Fun, now the test_bug732665.xul failures are back on win32 and win64 as well. Dave, mind if I piggyback on cypress for playing with the stack size? Shu gave me pointers on how to do it.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16) > Fun, now the test_bug732665.xul failures are back on win32 and win64 as > well. Dave, mind if I piggyback on cypress for playing with the stack size? > Shu gave me pointers on how to do it. Well if we're both trying to solve the same problem on the same tree we might end up solving it and not know which is the right solution. But if you have a better general solution (my fix would only affect add-ons manager code) then I guess that is the better solution anyway.
Created attachment 8606566 [details] [diff] [review] patch This is kind of the start of bug 987512 but for now it only touches AddonManagerInternal.getAddonByID and AddonManagerInternal.getAddonsByIDs converting them both to be entirely promise based. The public API is unaffected for now. This vastly reduces the stack used by calling getAddonsByIDs and also has the benefit of actually making the async calls run in parallel so may speed some operations up at the cost of doing some work that sometimes wouldn't have happened before. Verified on cypress that this solves the test failures.
Alas, bumping the stack size didn't fix this issue.
Fun observation - these failures only hit on dep builds. The win64 nightlies are fine.
Comment on attachment 8606566 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: Unknown [User impact if declined]: We might be seeing metadata updates for add-ons fail for users who have limited memory available [Describe test coverage new/current, TreeHerder]: The functions changed are probably the most well tested functions in the add-ons manager, many many automated tests call these functions in various ways and verify the results [Risks and why]: This does change the exact timing of the asynchronous functions but no code should be relying on that. This should be low risk. [String/UUID change made/needed]: None
Comment on attachment 8606566 [details] [diff] [review] patch Approved for uplift to aurora; has test coverage.