I've recently moved from the m31 release code base to m35 release code base for my little local customization project. I ran into many easily-reproducible debug assertions on m31. After spending only a couple of days on m35, I've run into quite a few debug assertions as well. I have a strong feeling that the debug build is not adequately tested by real-world actions from real users.Can we at least enable the DCHECK's in the canary build, assuming it is not already enabled? Exposing the dchecks to the outside world can help reveal a lot of potential problems in the code. I am not exactly sure how many people are downloading and playing with the canary build. If it's only limited to a couple of thousands of developers, I'd suggesting putting into the beta build instead.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
If people are building in release mode to avoid the DCHECKs I mentioned, then they are being lazy. If they are seeing those failures, they need to fix them not paper over them. That's why they are there. So I'm not really sure where you're going with this.
On Wed, Jun 4, 2014 at 3:50 PM, Scott Hess <sh...@chromium.org> wrote:
If people are building in release mode to avoid the DCHECKs I mentioned, then they are being lazy. If they are seeing those failures, they need to fix them not paper over them. That's why they are there. So I'm not really sure where you're going with this.If some completely unrelated area of the codebase has a DCHECK that fires when I do simple things like navigate to google.com, then I, too, would be tempted to just build in Release so that I can do work. What more can I do about that DCHECK than file a bug? I was not referring to the sql/ DCHECKs in particular (and I don't know that I've ever hit one of those), but I know that there are people building in Release because of failing DCHECKs.
That being said, I agree that they we can't really just turn them on in canary because it would be too crashy. Here's some ideas for getting the same benefit in a more user friendly way:Upload a crashdump without actually crashingOnly crash/upload with some small probabilityEnable DCHECKs with some small probability on process start (this would be better than enabling it for some % of installs so that no one user is stuck with the crashy version)
I have little confidence in the correctness of a number of DCHECKs. That is, the code is correct, but the DCHECK is wrong. While "good" to fix - I feel like that will only make for an extremely crashy, unreliable experience for users - even more so than the signals we use for Canary.See https://code.google.com/p/chromium/issues/detail?id=378079 for a real world example of good code, bad DCHECK.
--
From: Anthony LaForgeDate: 2014-06-04 20:30To: Luke Nihlen; ananthaSubject: Re: [chromium-dev] Enabling DCHECK's in canary or beta build
Sorry for the ignorance: what's a chromebot run?
> wrote: > A chromebot run wouldn't come close to real test by real users. We are > already running a lot of unit tests on the debug build already, aren't we? > But we are still missing out on so many assertions. > One very obvious bug I would like to mention as an example is the one I > just found. I download any file in a non-English UI such as Chinese, korean > and Japanese and I ran into a debug assertion immediately. It is not a > weird test case by any standar. However, the bug has been there since > m34, bypassing all the automatic tests. It would have been reported by > users on day 1 if dcheck is enabled on canary. > > I strongly support enabling dchecks in canary. And there are many ways we > can work out to do that without making the canary build totally unusable. > The most straightforward way is run a random number. If the random number > is within the range, we will throw a warning and let users to choose to > submit the bug report. We can choose 1% or 0.1% or any other percentage > number suitable for the team and the people outside. However, we MUST > expose the dchecks to real users so that we can eliminate most dchecks in > frequently-walked code path or caused by popular sites. > > ------------------------------ > Shanfeng Cheng > > > *From:* Anthony LaForge <laf...@chromium.org> > *Date:* 2014-06-04 20:30 > *To:* Luke Nihlen <lu...@chromium.org>; anantha <ana...@chromium.org> > *CC:* Peter Kasting <pkas...@chromium.org>; Ryan Sleevi > <rsl...@chromium.org>; João da Silva <joaod...@chromium.org>; Shanfeng > Cheng <sfc...@gmail.com>; kareng <kar...@chromium.org>; Chromium-dev > <chromi...@chromium.org> > *Subject:* Re: [chromium-dev] Enabling DCHECK's in canary or beta build > The premise is interesting, although let's perhaps re-direct that energy a > little bit. Rather than inflict this pain (i.e. we have a high confidence > that this will be unstable), alternatively, perhaps a better initial target > would be a chromebot run of a DCHECK enabled build. I'm guessing that > would shake out a fair amount of low hanging fruit, w/ out directly > inconveniencing our user base (who are much less tolerant of instability > than most would suspect). > > Kind Regards, > > Anthony Laforge > Technical Program Manager > Mountain View, CA > > > On Wed, Jun 4, 2014 at 6:11 PM, Luke Nihlen <lu...@chromium.org> wrote: > >> I would volunteer to be part of a DCHECK cleanup squad should we decide >> to turn this on in Canary. Which I think we should. >> >> \L >> >> >> On Wed, Jun 4, 2014 at 6:07 PM, Peter Kasting <pkas...@chromium.org> >> wrote: >> >>> On Wed, Jun 4, 2014 at 1:57 PM, Ryan Sleevi <rsl...@chromium.org> >>> wrote: >>> >>>> I have little confidence in the correctness of a number of DCHECKs. >>>> That is, the code is correct, but the DCHECK is wrong. While "good" to fix >>>> - I feel like that will only make for an extremely crashy, unreliable >>>> experience for users - even more so than the signals we use for Canary. >>>> >>>> See https://code.google.com/p/chromium/issues/detail?id=378079 for a >>>> real world example of good code, bad DCHECK. >>>> >>> >>> I strongly disagree with this argument. I also disagree with how the >>> network team seems to be handling the bug you link (i.e. not treating this >>> as important to fix). >>> >>> The history of our policy toward DCHECKs is that in the Firefox days, >>> there were all kinds of assertions that failed and dumped output to the >>> console, but because there were hundreds, no one worried about them, or >>> noticed when more happened. This meant that any real bugs they highlighted >>> didn't get fixed, and code that asserted couldn't be trusted to actually be >>> run with the assertions holding. >>> >>> We didn't want any of those things to be true of Chrome, so we created a >>> very clear policy. DCHECK failure must not be handled except in the case >>> where not handling it would lead to an exploitable security bug. All >>> DCHECKs must pass, and should be read as strong assertions that they will >>> _never_ fail, even in error cases -- things like full memory or disk >>> corruption should never lead to failing DCHECKs. Encountering a DCHECK >>> failure should be treated exactly like encountering a browser crash: it is >>> a P1 bug and should be fixed immediately. >>> >>> If there are DCHECKs in the code today for which this is not true, it >>> undermines all the advantages this sort of strict policy gives. We don't >>> want that to happen. Therefore, we SHOULD run things like Canary with >>> DCHECKs enabled, it should not cause excessive crashing, and if it does, we >>> should fix those crashes with high priority. We should never say that >>> because there might be incorrect DCHECKs, we should avoid running Canary >>> with DCHECKs on. >>> >>> If there are subteams not handling DCHECKs according to the policy >>> above, please correct things. Make sure people take DCHECK failure as >>> seriously as crashing. Disallow most instances of the following >>> constructions in code you write or review: >>> >>> DCHECK(foo); >>> if (!foo) ... >>> >>> or >>> >>> if (foo) { >>> NOTREACHED(); >>> return; >>> } >>> >>> The benefit of this will be safer, more readable code. >>> >>> PK >>> >>> -- >>> -- >>> Chromium Developers mailing list: chromi...@chromium.org
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
On Jun 4, 2014 11:16 PM, "Shanfeng Cheng" <sfc...@gmail.com> wrote:
>
> Let's say there are 100 known bugs and 100 people working on them with their hands full. There are a simple switch we can flip so that we can know another 900 bugs. I'd still rather flip the switch to at least know all the bugs. At least, I can pick the most important 100 bugs out of the 1000 to fix with the people we have. We still get a higher quality project by doing that.
>
Wouldn't the 100 known bugs - the ones being encountered by real users in the real world, as evidenced by the crash data - by definition be the most important?
From: Ryan SleeviDate: 2014-06-05 01:31To: Shanfeng ChengSubject: Re: Re: [chromium-dev] Enabling DCHECK's in canary or beta build
On Jun 4, 2014 11:16 PM, "Shanfeng Cheng" <sfc...@gmail.com> wrote:
>
> Let's say there are 100 known bugs and 100 people working on them with their hands full. There are a simple switch we can flip so that we can know another 900 bugs. I'd still rather flip the switch to at least know all the bugs. At least, I can pick the most important 100 bugs out of the 1000 to fix with the people we have. We still get a higher quality project by doing that.
>
What is the goal, though? If the solution to a DCHECK firing on a canary of this sort is to remove the DCHECK, then we really haven't gained anything. If the solution is to analyze the reason for the DCHECK and fix the code, that would be great. But ... do you not have enough known bugs to fix currently?
I think enable DCHECKs on Canary is just a recipe for further reducing Canary usage.
As a for-instance, I've run across DCHECKs which seem to be threading-related and are hard to repro, so they _probably_ indicate a problem, but it's hard to figure out what they're telling me.
I have little confidence in the correctness of a number of DCHECKs. That is, the code is correct, but the DCHECK is wrong. While "good" to fix - I feel like that will only make for an extremely crashy, unreliable experience for users - even more so than the signals we use for Canary.See https://code.google.com/p/chromium/issues/detail?id=378079 for a real world example of good code, bad DCHECK.
On Wed, Jun 4, 2014 at 1:53 PM, Joao da Silva <joaod...@chromium.org> wrote:
I support this idea based on recent bugs I've looked at, that would have been easily spotted by a DCHECK in the dev channel on ChromeOS.Has a conscious decision been made about disabling DCHECKs in the canary and dev channels? Enabling them would trigger more crashes but would also generate useful reports.
On Tue, Jun 3, 2014 at 11:25 PM, Stephen Cheng <sfc...@gmail.com> wrote:
I've recently moved from the m31 release code base to m35 release code base for my little local customization project. I ran into many easily-reproducible debug assertions on m31. After spending only a couple of days on m35, I've run into quite a few debug assertions as well. I have a strong feeling that the debug build is not adequately tested by real-world actions from real users.Can we at least enable the DCHECK's in the canary build, assuming it is not already enabled? Exposing the dchecks to the outside world can help reveal a lot of potential problems in the code. I am not exactly sure how many people are downloading and playing with the canary build. If it's only limited to a couple of thousands of developers, I'd suggesting putting into the beta build instead.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
If there are subteams not handling DCHECKs according to the policy above, please correct things. Make sure people take DCHECK failure as seriously as crashing. Disallow most instances of the following constructions in code you write or review:
DCHECK(foo);if (!foo) ...orif (foo) {NOTREACHED();return;}
One more thing: Could somebody please offer guidance as to when to use CHECK() and when DCHECK()? Is it just a matter of avoiding CHECK() in hotspots or do we generally prefer DCHECK() because of code size considerations?
What is the goal, though? If the solution to a DCHECK firing on a canary of this sort is to remove the DCHECK, then we really haven't gained anything. If the solution is to analyze the reason for the DCHECK and fix the code, that would be great. But ... do you not have enough known bugs to fix currently?Like I said earlier, if developers are running with DCHECKs disabled because they cannot be bothered to log and/or fix bugs, then those developers are frankly letting the team down. Enabling DCHECKs on canary won't fix that, it will just generate a bunch of un-triaged reports to overwhelm the relatively small number of people who spend their time triaging crash reports.