Regression fixing

[Aleksey Bragin]

Hello,
as you all know we have quite a lot of regressions recently, and  
recently they only add up to one another causing annoyances of  
testers and developers. There is a strong need to change this  
situation as soon as possible, otherwise the project's future is  
undetermined.

I want to propose a step-by-step approach. Our brave testing team has  
created a good overview of the most important regressions and bugs we  
have so far: http://www.reactos.org/wiki/Buglist .

Now, very important(!):
For the first step I would like ALL developers to drop their current  
ReactOS-related work, including all work in branches or wherever else  
and focus ONLY on fixing regressions from that list. The goal is to  
fix all confirmed regressions that have been introduced, starting  
from the most recent and going down to the most ancient. All possible  
ways to remove a regression could be used: starting from a proper fix  
and finishing with a total revert or commenting out even good code.

Process coordination: feel free to commit proper fixes right away,  
however as for reverts, I, and/or comittee of our core devs, would  
like to have a final say on whether something should be reverted.

I repeat, all other non-regression related commits to the official  
ReactOS SVN repository are forbidden, even to branches. The only  
exception may be  developers whose access is restricted to branches  
only.


Thank you for understanding,
Aleksey Bragin.

[victor martinez]

I have my RosBE and my VBOX ready.

I will join any regression fix attempt. If anyone needs me to test a patch or wants my
testing help, just has to contact me via IRC.
This is a Captain Obvious message.



 		 	   		  
_________________________________________________________________
¡Citas! ¡Ligues! ¿Salimos? ¿Cómo es tu pareja ideal? Búscala en el sitio nº1… ¡Regístrate
ya!
http://contactos.es.msn.com/?mtcmk=015352

[Javier Agustìn Fernàndez Arroyo]

im waiting for being able to test again :P so i can test any patch too...
just, you will have to teach me how to apply patches :D

On Wed, Apr 28, 2010 at 3:32 PM, victor martinez <vicmarcal(a)hotmail.com>wrote;wrote:

>    I have my RosBE and my VBOX ready.
>
>  I will join any regression fix attempt. If anyone needs me to test a patch
>  or wants my testing help, just has to contact me via IRC.
>  This is a Captain Obvious message.
>
>
>
>
>  ------------------------------
>  Tus datos personales, más seguros con Internet Explorer 8. ¡Descárgatelo
>  gratis!<http://www.microsoft.com/spain/windows/internet-explorer/default…
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>

[James Tabor]

Amazing!

On Wed, Apr 28, 2010 at 7:41 AM, Aleksey Bragin <aleksey(a)reactos.org> wrote:
>   Hello,
>  as you all know we have quite a lot of regressions recently, and recently
>  they only add up to one another causing annoyances of testers and
>  developers. There is a strong need to change this situation as soon as
>  possible, otherwise the project's future is undetermined.
>
>  I want to propose a step-by-step approach. Our brave testing team has
>  created a good overview of the most important regressions and bugs we have
>  so far: http://www.reactos.org/wiki/Buglist .
>
>  Now, very important(!):
>  For the first step I would like ALL developers to drop their current
>  ReactOS-related work, including all work in branches or wherever else and
>  focus ONLY on fixing regressions from that list. The goal is to fix all
>  confirmed regressions that have been introduced, starting from the most
>  recent and going down to the most ancient. All possible ways to remove a
>  regression could be used: starting from a proper fix and finishing with a
>  total revert or commenting out even good code.
>
>  Process coordination: feel free to commit proper fixes right away, however
>  as for reverts, I, and/or comittee of our core devs, would like to have a
>  final say on whether something should be reverted.
>  
Oh yes that works! No one asked me first, even after posting to debug
logs before the reverts!

Like I've posted before, reverting still does not fix the issue since
the issue was not fixed in the first place!

>   I repeat, all other non-regression related commits to
> the official ReactOS
>  SVN repository are forbidden, even to branches. The only exception may be
>   developers whose access is restricted to branches only.
>
>
>  Thank you for understanding,
>  Aleksey Bragin.
>  
Win32k:
If you really want to fix this mess! First do some research, assume
the code that was working before it was broken is correct. Trust me
this does works from trial and error! Find out from your research what
caused the breakage, that would be from data reduction testing by
revision to revision. Instead of reverting, try something new, like
fixing it. I know it is very hard work and this was what I was doing
for ten years here but to do it right you have to do research and
study. Example: wine...

Reverting things retrogrades the project since those changes are now
lost. Lost information is always lost and never comes back. Not in the
source is still not in the source.

Good Luck!

[James Tabor]

"Well, let's clear it up once and for all. I don't want this topic to
be brought again and again."

On Wed, Apr 28, 2010 at 7:41 AM, Aleksey Bragin <aleksey(a)reactos.org> wrote:
>   Hello,
>  Now, very important(!):
>  For the first step I would like ALL developers to drop their current
>  ReactOS-related work, including all work in branches or wherever else and
>  focus ONLY on fixing regressions from that list. 
After communicating on IRC and analyzing the new development model
with the use of multiple branches. ReactOS need to revert this new
model and go back to doing the development in one main branch. Most of
our developers have more than one branch and spending more time with
those branches. Obviously this is not working since this issue keeps
coming up every month. Brain freeze trunking! Lousing track on which
branch that has regressed in the debug logs is a big problem and is
growing. Instead of working outward or away from each other, ReactOS
needs to work back inside and start merging back in. Work out the new
issues and stay inward and focus on one branch.

Good Luck!

[Olaf Siejka]

The most important reason for moving development into branches was to
minimize the effect of prolonged trunk breakage, that has to happen with
rewrites. This is a vital issue, as inability of testing trunk on the daily
basis is very often a seed for regression accumulation.

2010/4/29 James Tabor <jimtabor.rosdev(a)gmail.com>

>   "Well, let's clear it up once and for all. I
> don't want this topic to
>  be brought again and again."
>
>  On Wed, Apr 28, 2010 at 7:41 AM, Aleksey Bragin <aleksey(a)reactos.org>
>  wrote:
> >   Hello,
> >  Now, very important(!):
> >  For the first step I would like ALL developers to drop their current
> >  ReactOS-related work, including all work in branches or wherever else and
> >  focus ONLY on fixing regressions from that list. 
>  After communicating on IRC and analyzing the new development model
>  with the use of multiple branches. ReactOS need to revert this new
>  model and go back to doing the development in one main branch. Most of
>  our developers have more than one branch and spending more time with
>  those branches. Obviously this is not working since this issue keeps
>  coming up every month. Brain freeze trunking! Lousing track on which
>  branch that has regressed in the debug logs is a big problem and is
>  growing. Instead of working outward or away from each other, ReactOS
>  needs to work back inside and start merging back in. Work out the new
>  issues and stay inward and focus on one branch.
>
>  Good Luck!
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>

[James Tabor]

Hum?

On Thu, Apr 29, 2010 at 5:45 PM, Olaf Siejka <caemyr(a)gmail.com> wrote:
>   The most important reason for moving development into
> branches was to
>  minimize the effect of prolonged trunk breakage, that has to happen with
>  rewrites. This is a vital issue, as inability of testing trunk on the daily
>  basis is very often a seed for regression accumulation.
>  
If we had one hundred or more full time developers, this might work.
We don't so it's not working and it is creating a mess and losing the
little time we have in branches. The argument that was pushed and
posted by the developers that left the project recently objected to
dividing resources in regards of the other project branch (which
should have remained as a research project and not the official
replacement of a ReactOS subsystem) thus proving them correct.

Look at this as a signal warning before the shit hits the reef.......
The ship has turn into the reef!

[Olaf Siejka]

Then again, how would you propose to deal with regressions that will for
sure slip in at periods of time, when trunk is not buildable or doesnt boot?

For example, inbetween ac97 driver install breakage range - rev: 46977-47057
i counted at least 4 regressions that were introduced in revisions
inbetween.

2010/4/30 James Tabor <jimtabor.rosdev(a)gmail.com>

>   Hum?
>
>  On Thu, Apr 29, 2010 at 5:45 PM, Olaf Siejka <caemyr(a)gmail.com> wrote:
> >   The most important reason for moving development
> > into branches was to
> >  minimize the effect of prolonged trunk breakage, that has to happen with
> >  rewrites. This is a vital issue, as inability of testing trunk on the 
>  daily
> >   basis is very often a seed for regression
> > accumulation.
> >  
>  If we had one hundred or more full time developers, this might work.
>  We don't so it's not working and it is creating a mess and losing the
>  little time we have in branches. The argument that was pushed and
>  posted by the developers that left the project recently objected to
>  dividing resources in regards of the other project branch (which
>  should have remained as a research project and not the official
>  replacement of a ReactOS subsystem) thus proving them correct.
>
>  Look at this as a signal warning before the shit hits the reef.......
>  The ship has turn into the reef!
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>

[Aleksey Bragin]

On Apr 30, 2010, at 3:51 AM, James Tabor wrote:

>   On Thu, Apr 29, 2010 at 5:45 PM, Olaf Siejka
> <caemyr(a)gmail.com> wrote:
> >   The most important reason for moving development
> > into branches was to
> >  minimize the effect of prolonged trunk breakage, that has to  
> >  happen with
> >  rewrites. This is a vital issue, as inability of testing trunk on  
> >  the daily
> >  basis is very often a seed for regression accumulation.
> >  
>  If we had one hundred or more full time developers, this might work. 
It's
important to be ready right now, not in some distant future  
which may never come. We have quite a few developers already. It's  
just a matter of time when we reach 100 devs, 200 devs, etc milestones.

>   We don't so it's not working and it is
> creating a mess and losing the
>  little time we have in branches. The argument that was pushed and
>  posted by the developers that left the project recently objected to
>  dividing resources in regards of the other project branch (which
>  should have remained as a research project and not the official
>  replacement of a ReactOS subsystem) thus proving them correct. 
Please be specific,
no water. Who left and why?
ReactOS is an international project, and it has to be managed. If you  
don't like the existing way - try to do better, but be a teamplayer  
like you always were in our team. I don't know what happened to you  
in the last year.

Have a look at the successful, very big (bigger than ours) project -  
KDE. This is from their Code of Conduct (Capt. Obvious skips the "Be  
respectful" part), please ready very thoroughly:
"Be pragmatic
KDE is a pragmatic community. We value tangible results over having  
the last word in a discussion. We defend our core values like freedom  
and respectful collaboration, but we don't let arguments about minor  
issues get in the way of achieving more important results. We are  
open to suggestions and welcome solutions regardless of their origin.  
When in doubt support a solution which helps getting things done over  
one which has theoretical merits, but isn't being worked on. Use the  
tools and methods which help getting the job done. Let decisions be  
taken by those who do the work."

This works for them, and is reasonable. They target real result, less  
amount of bugs and less regressions. They are not afraid of doing  
bugfixing weeks, regression chasing. If you are unhappy even with so  
small(!!) step I just did, I strongly suggest to carefully think, and  
review your earlier magnificent contribution to ReactOS project.

I will repeat myself, I don't recognize you. Previously, in early  
years, you were spending days and weeks testing stuff before  
commiting, and it worked really well. Now you just fire off the  
commit and blame the kernel in any possible bug. That's not how our  
team is going to work.


>   Look at this as a signal warning before the shit hits
> the reef.......
>  The ship has turn into the reef! 
No need to give false signals here, they often
lead to loss of  
investment (example from stock trading area).


WBR,
Aleksey.

[James Tabor]

On Fri, Apr 30, 2010 at 8:26 AM, Aleksey Bragin <aleksey(a)reactos.org> wrote:
>  
>  I will repeat myself, I don't recognize you. Previously, in early years, you
Dude! I haven't changed! You have changed a lot and I don't know you
anymore.

>   were spending days and weeks testing stuff before
> commiting, and it worked
>  really well. Now you just fire off the commit and blame the kernel in any
>  possible bug. That's not how our team is going to work.
>  
I'm reacting to when it gets broken! I could be wrong and when I am I
do admit it openly! But, you need to read bug 5265. I guess we can
revert all the gdi batch code to fix it. Yes it fixes it but still the
real problem persists, what happen to the TEB?

http://www.reactos.org/bugzilla/show_bug.cgi?id=5265

This bug is due to a hax fix I did to help 5265!
http://www.reactos.org/bugzilla/show_bug.cgi?id=5314

Might I add, no one has followed up on any of my suggestions.

>  
> >   Look at this as a signal warning before the ship
> > hits the reef.......
> >  The ship has turn into the reef! 
>  No need to give false signals here, they often lead to loss of investment
>  (example from stock trading area). 
Trying to make money from ReactOS is a bad idea
and thinking about
money from ReactOS, makes this idea delusional.

[Javier Agustìn Fernàndez Arroyo]

IMHO Aleksey means that ReactOS needs a money invest for a big jump in its
progress...

On Sat, May 1, 2010 at 5:57 AM, James Tabor <jimtabor.rosdev(a)gmail.com>wrote;wrote:

>   On Fri, Apr 30, 2010 at 8:26 AM, Aleksey Bragin
> <aleksey(a)reactos.org>
>  wrote:
> >  
> >  I will repeat myself, I don't recognize you. Previously, in early years, 
>  you
>  Dude! I haven't changed! You have changed a lot and I don't know you
>  anymore.
>
> >   were spending days and weeks testing stuff before
> > commiting, and it 
>  worked
> >   really well. Now you just fire off the commit and
> > blame the kernel in any
> >  possible bug. That's not how our team is going to work.
> >  
>  I'm reacting to when it gets broken! I could be wrong and when I am I
>  do admit it openly! But, you need to read bug 5265. I guess we can
>  revert all the gdi batch code to fix it. Yes it fixes it but still the
>  real problem persists, what happen to the TEB?
>
>  http://www.reactos.org/bugzilla/show_bug.cgi?id=5265
>
>  This bug is due to a hax fix I did to help 5265!
>  http://www.reactos.org/bugzilla/show_bug.cgi?id=5314
>
>  Might I add, no one has followed up on any of my suggestions.
>
> >  
> > >   Look at this as a signal warning before the ship
> > > hits the reef.......
> > >  The ship has turn into the reef! 
> >  No need to give false signals here, they often lead to loss of investment
> >  (example from stock trading area). 
>  Trying to make money from ReactOS is a bad idea
> and thinking about
>  money from ReactOS, makes this idea delusional.
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>

[Aleksey Bragin]

On May 1, 2010, at 7:57 AM, James Tabor wrote:
>   I'm reacting to when it gets broken! I could be
> wrong and when I am I
>  do admit it openly! But, you need to read bug 5265. I guess we can
>  revert all the gdi batch code to fix it. Yes it fixes it but still the
>  real problem persists, what happen to the TEB? 
This is exactly the problematic
behavior: Why enable GDI batch code  
if it does NOT work?  Fix underlying bug *first* and then enable the  
code. Especially considering that GDI batch may be seen as  
optimization, not as a vital code needed for rendering. Is it?

And honestly, you are trying to find a bug in the wrong direction, in  
my opinion.
Analyze it logically: somehow, TEB appears to work everywhere except  
for your GDI batch code. And, one of the hackfixs you committed in  
46414 is around this code:
        for (; GdiBatchCount > 0; GdiBatchCount--)
        {
            ULONG Size;
             // Process Gdi Batch!
            Size = GdiFlushUserBatch(pDC, (PGDIBATCHHDR) pHdr);
            if (!Size) break;
            pHdr += Size;
         }
You "protected" pHdr pointer dereference inside GdiFlushUserBatch()  
by wrapping it into SEH.
Now let's call Captain Obvious again to help and think, what is more  
probable: Mysterious TEB invalidation, or GdiBatchCount not matching  
the size of pHdr array? So that the reading goes beyond the buffer  
and certainly faults (and you hack-catch this fault in SEH to pretend  
you caught the mysterious kernel bug).
I suspect the latter, because it might be anything - race condition,  
a bug in the code, a buffer overflow, uninitialized variable access,  
or you're trying to flush the batch queue of an already dead thread.

That's just a glance over the bug, without even looking to the code -  
just reading the diff was enough.


>   http://www.reactos.org/bugzilla/show_bug.cgi?id=5265
>
>  This bug is due to a hax fix I did to help 5265!
>  http://www.reactos.org/bugzilla/show_bug.cgi?id=5314
>
>  Might I add, no one has followed up on any of my suggestions. 
That's why you
decided to crash in the non-working code and wait for  
people's reaction? Not good.

>  
> >  
> > >   Look at this as a signal warning before the ship
> > > hits the  
> > >  reef.......
> > >  The ship has turn into the reef! 
> >  No need to give false signals here, they often lead to loss of  
> >  investment
> >  (----->example from stock trading area<-----). 
>  Trying to make money from
> ReactOS is a bad idea and thinking about
>  money from ReactOS, makes this idea delusional. 
Your weed is very nice, please
share.

[Timo Kreuzer]

Aleksey Bragin wrote:
>  
>  On May 1, 2010, at 7:57 AM, James Tabor wrote:
> >   I'm reacting to when it gets broken! I could
> > be wrong and when I am I
> >  do admit it openly! But, you need to read bug 5265. I guess we can
> >  revert all the gdi batch code to fix it. Yes it fixes it but still the
> >  real problem persists, what happen to the TEB? 
>  This is exactly the problematic
> behavior: Why enable GDI batch code if
>  it does NOT work?  Fix underlying bug *first* and then enable the
>  code. Especially considering that GDI batch may be seen as
>  optimization, not as a vital code needed for rendering. Is it?
>
>  And honestly, you are trying to find a bug in the wrong direction, in
>  my opinion.
>  Analyze it logically: somehow, TEB appears to work everywhere except
>  for your GDI batch code. And, one of the hackfixs you committed in
>  46414 is around this code:
>         for (; GdiBatchCount > 0; GdiBatchCount--)
>         {
>             ULONG Size;
>              // Process Gdi Batch!
>             Size = GdiFlushUserBatch(pDC, (PGDIBATCHHDR) pHdr);
>             if (!Size) break;
>             pHdr += Size;
>          }
>  You "protected" pHdr pointer dereference inside GdiFlushUserBatch() by
>  wrapping it into SEH.
>  Now let's call Captain Obvious again to help and think, what is more
>  probable: Mysterious TEB invalidation, or GdiBatchCount not matching
>  the size of pHdr array? So that the reading goes beyond the buffer and
>  certainly faults (and you hack-catch this fault in SEH to pretend you
>  caught the mysterious kernel bug).
>  I suspect the latter, because it might be anything - race condition, a
>  bug in the code, a buffer overflow, uninitialized variable access, or
>  you're trying to flush the batch queue of an already dead thread.
>  
 
Both broken fonts and the massive gdi object leak in Acrobat reader and
other apps and probably a bazillion of other bugs were caused by broken
pointer arithmetics in gdi32 batch code. ZWabbit knows what I mean. And
I would also suspect it to be at fault when "the teb is missing", The
code doesn't protect against malicious values in the Size field of the
headers and when you use the upper half of a handle instead of the size
value ...

I'm fixing it. But after fixing the gdi object leak, Acrobat reader
doesn't die a painful death due to too many objects, but some objects
seem to be missing, causing broken rendering. Hiding a bug with another
bug. Ironic, isn't it?

Regards,
Timo

[James Tabor]

Good job!
Looks like Timo is fixing it and Michael is fixing things too. Thanks
to all of you for helping me! I'll be around later when my health gets
better.
8^)

On Sat, May 1, 2010 at 3:53 AM, Aleksey Bragin <aleksey(a)reactos.org> wrote:
>  
>
>  Your weed is very nice, please share.
>  
Now you! You are nuts (GreatLordish -> NOTS), you need to stop smoking
that opium the US army is shipping up to Russia from Afghanistan!

Thanks again everyone see you soon,
James


Oh yeah~ Good Luck!

[Aleksey Bragin]

The regression fixing period is quite nice, quite a few of  
regressions were fixed (maybe someone could do a nice stats based on  
bugzilla?), so the teamwork really brings a lot.

De-facto, we already enhanced borders of this regfixing period to  
include very important bug fixes. I would like to make it official,  
however, be very careful and fix only those bugs which are less  
likely to introduce a new regression. And still, regressions are the  
top priority! Consider them as a blocker for coming 0.3.12 release.  
It will be nice to finally start releasing without regressions, isn't  
it? :)

Important note: I'm leaving for Sweden for a business trip tomorrow,  
which is going to last about 5 days. I will join the IRC channel  
occasionally and check emails of course, but nothing more than that.

Keep up the great work!

WBR,
Aleksey Bragin.

On Apr 28, 2010, at 4:41 PM, Aleksey Bragin wrote:

>   Hello,
>  as you all know we have quite a lot of regressions recently, and  
>  recently they only add up to one another causing annoyances of  
>  testers and developers. There is a strong need to change this  
>  situation as soon as possible, otherwise the project's future is  
>  undetermined.
>
>  I want to propose a step-by-step approach. Our brave testing team  
>  has created a good overview of the most important regressions and  
>  bugs we have so far: http://www.reactos.org/wiki/Buglist .
>
>  Now, very important(!):
>  For the first step I would like ALL developers to drop their  
>  current ReactOS-related work, including all work in branches or  
>  wherever else and focus ONLY on fixing regressions from that list.  
>  The goal is to fix all confirmed regressions that have been  
>  introduced, starting from the most recent and going down to the  
>  most ancient. All possible ways to remove a regression could be  
>  used: starting from a proper fix and finishing with a total revert  
>  or commenting out even good code.
>
>  Process coordination: feel free to commit proper fixes right away,  
>  however as for reverts, I, and/or comittee of our core devs, would  
>  like to have a final say on whether something should be reverted.
>
>  I repeat, all other non-regression related commits to the official  
>  ReactOS SVN repository are forbidden, even to branches. The only  
>  exception may be  developers whose access is restricted to branches  
>  only.
>
>
>  Thank you for understanding,
>  Aleksey Bragin.