Console Csrss Handle Errors

[James Tabor]

Hi all,
Bored again, decided to build ros on ros.

Getting this,

(subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
(KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
(KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
(subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
(subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle

Things I noticed may have been fixed,
Multi cmd problem, handle count going throw the roof and hangs.

Thanks,
James

[Hartmut Birr]

James Tabor wrote:
> Hi all,
> Bored again, decided to build ros on ros.
>
> Getting this,
>
> (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
> (KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
> (KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
> (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
> (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle

That isn't a real problem in csrss. Csrss checks for a valid console
handle and prints a message if the handle is invalid. The problem is in
CreateProcess. The handling of the stderr handle is wrong.

- Hartmut

[Alex Ionescu]

Hartmut Birr wrote:
> James Tabor wrote:
> > Hi all,
> > Bored again, decided to build ros on ros.
> >
> > Getting this,
> >
> > (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
> > (KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
> > (KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
> > (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
> > (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
>
> That isn't a real problem in csrss. Csrss checks for a valid console
> handle and prints a message if the handle is invalid. The problem is in
> CreateProcess. The handling of the stderr handle is wrong.

Hi,

Please, I beg you, do not commit any changes to CreateProcess now. I 
have a 10 000+ line patch related to CreateProcess that I'm going to 
commit today or tomorrow, and the conflict would kill me. Thank you for 
your understanding.

Best regards,
Alex Ionescu
> Hartmut
>
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

[Casper Hornstrup]

I thought we already established that such big patches are not good for us.

Casper
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Alex Ionescu
> Sent: 11. juli 2005 20:40
> To: ReactOS Development List
> Subject: Re: [ros-dev] Console Csrss Handle Errors
>
> Hi,
>
> Please, I beg you, do not commit any changes to CreateProcess now. I
> have a 10 000+ line patch related to CreateProcess that I'm going to
> commit today or tomorrow, and the conflict would kill me. Thank you for
> your understanding.
>
> Best regards,
> Alex Ionescu

[Alex Ionescu]

Casper Hornstrup wrote:
> I thought we already established that such big patches are not good for us.

No, we established that misc branches and patches containing 100 
unrelated fixes are not good for us.

I have not yet seen how large patches which are specific to a single 
feature/fix, and which come with extremly detailed changelogs and 
separated .diff files for easy reading are bad for us. Let me know if 
you've decided this new policy, I haven't seen a vote on it.

Best regards,
Alex Ionescu
> Casper
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On Behalf Of Alex Ionescu
> > Sent: 11. juli 2005 20:40
> > To: ReactOS Development List
> > Subject: Re: [ros-dev] Console Csrss Handle Errors
> >
> > Hi,
> >
> > Please, I beg you, do not commit any changes to CreateProcess now. I
> > have a 10 000+ line patch related to CreateProcess that I'm going to
> > commit today or tomorrow, and the conflict would kill me. Thank you for
> > your understanding.
> >
> > Best regards,
> > Alex Ionescu
>
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

[Casper Hornstrup]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Alex Ionescu
> Sent: 11. juli 2005 21:33
> To: ReactOS Development List
> Subject: Re: [ros-dev] Console Csrss Handle Errors
>
> Casper Hornstrup wrote:
> > I thought we already established that such big patches are not good for us.
>
> No, we established that misc branches and patches containing 100
> unrelated fixes are not good for us.
>
> I have not yet seen how large patches which are specific to a single
> feature/fix, and which come with extremly detailed changelogs and
> separated .diff files for easy reading are bad for us. Let me know if
> you've decided this new policy, I haven't seen a vote on it.
>
> Best regards,
> Alex Ionescu
> > Casper

We can vote on it, but we've seen many arguments on the issue during the previous vote:
http://reactos.com/wiki/index.php/Voting/Miscelanea_branches

Issues like big patches are much harder to review and it's much harder to pinpoint regressions caused by big patches.

Casper

[Alex Ionescu]

Casper Hornstrup wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On Behalf Of Alex Ionescu
> > Sent: 11. juli 2005 21:33
> > To: ReactOS Development List
> > Subject: Re: [ros-dev] Console Csrss Handle Errors
> >
> > Casper Hornstrup wrote:
> > > I thought we already established that such big patches are not good for us.
> >
> > No, we established that misc branches and patches containing 100
> > unrelated fixes are not good for us.
> >
> > I have not yet seen how large patches which are specific to a single
> > feature/fix, and which come with extremly detailed changelogs and
> > separated .diff files for easy reading are bad for us. Let me know if
> > you've decided this new policy, I haven't seen a vote on it.
> >
> > Best regards,
> > Alex Ionescu
> > > Casper
>
> We can vote on it, but we've seen many arguments on the issue during the previous vote:
> http://reactos.com/wiki/index.php/Voting/Miscelanea_branches

I guess I wasn't clear in my email. This is not a misc patch/branch.
> Issues like big patches are much harder to review and it's much harder to pinpoint regressions caused by big patches.

So, should we revert rbuild? It was a pretty big patch too. And I can 
name a couple of other ones... (let's not get started into to how many 
regressions rbuild caused :)

I cannot please you Casper, I'm sorry. First I make small changelogs, 
and you complain they are too small. Then I make large ones, and you 
complain they are too big. Then you tell me to use branches, I do, and 
you complain that it was too broad/large, and that I should split up 
patches and work on single features too. Now I spend two weeks on a 
patch which targets a single thing, and you compain again. Would it be 
better for you if I just left the project? Nothing sucks more then 
spending 2 weeks working on something (And YES it has and will still be 
reviewed by other devs) to be told "I don't want that'. I find that 
disappointing.

Best regards,
Alex Ionescu
> Casper
>
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

[Casper Hornstrup]

> So, should we revert rbuild? It was a pretty big patch too. And I can
> name a couple of other ones... (let's not get started into to how many
> regressions rbuild caused :)

I gave you the opportunity to provide a list of revision numbers with
bugfixes for rbuild so we can see the real damages done by it and you
couldn't/didn't want to do that. It's impossible for me to counter your
"let's not get started into to how many regressions rbuild caused"
argument. I could never counter that strong argument.

However, if we had a list of bugfixes, we could calculate the number of
bugs per line of code in the rbuild patch. That is so much better since
we can actually use that to measure the success of the patch and to
improve the process, next time we develop such a big change.
> I cannot please you Casper, I'm sorry. First I make small changelogs,
> and you complain they are too small.

Not small. Very un-useful commit messages like "fix A". It would be very
useful for other developers to know what was actually changed.
> Then I make large ones, and you
> complain they are too big.

I don't know what you are referring to here.
> Then you tell me to use branches, I do, and
> you complain that it was too broad/large,
> and that I should split up
> patches and work on single features too.

Miscelanea branches, yes. Check. For more information on why they are bad look here:
http://reactos.com/wiki/index.php/Voting/Miscelanea_branches
> Now I spend two weeks on a
> patch which targets a single thing, and you compain again.

If it's a 10.000 line patch then I'm almost certain that only relation
there could be is that the code is involved in the CreateProcess control
path. Still no reason not to develop it incrementally IMO.
> Would it be
> better for you if I just left the project? Nothing sucks more then
> spending 2 weeks working on something (And YES it has and will still be
> reviewed by other devs) to be told "I don't want that'. I find that
> disappointing.
>
> Best regards,
> Alex Ionescu

I'm sorry that you always think I'm "out to get you" when I complain
about the way we do things and you are involved. I can't change how you
feel - unless maybe I never will complain about things in which you are
involved.

It's not like you countered my preliminary arguments with great arguments
for developing such big patches in your local working copy. Why don't you
do that instead of bringing up a whole bunch of other unrelated issues?
Convince me (and others) that such big patches are good for the project.

Casper

[Alex Ionescu]

Casper Hornstrup wrote:
> I'm sorry that you always think I'm "out to get you" when I complain
> about the way we do things and you are involved. I can't change how you
> feel - unless maybe I never will complain about things in which you are
> involved.
>
> It's not like you countered my preliminary arguments with great arguments
> for developing such big patches in your local working copy. Why don't you
> do that instead of bringing up a whole bunch of other unrelated issues?
> Convince me (and others) that such big patches are good for the project.
>
> Casper

I hope you appreciate the fact that I spent 4 extra hours attempting to 
split up this patch, which required me to add hacks to the unpatched 
parts to make them compile/work, only to over-write that code 5 minutes 
later. So there is argument number one. Splitting up patches made me 
lose time, and possibly screw things up.

Secondly, you don't seem to have addressed the fact that svn diffs are 
not optimized. Nor that there have been large patches by other people 
which were accepted perfectly. I've seen patches were a developer 
deletes 10 files and re-writes a 500 line file. That can easily cause a 
5000+ line patch. There is no way that will cause more regressions or 
break the tree then if the patch were only the ~1000 lines needed. 3000 
lines of the 10K number were deletions. Renamings are annoying too. 
Another 2K lines of my patch was simply renaming all the code that uses 
members in a structure which was named incorrectly. Once again, no real 
code change. Just another artificial inflation. What possible good point 
was there in me comitting this part of my patch separately? I had to 
edit parts of /lib/rtl that the next patch totally removed!

Thirdly, patches themselves can be big. My actual CreateProcess patch is 
about 2-3K lines, which is still "large". There is no way it can be 
split up more. You will just have to face the fact.

Most people/places would be very proud that a coder spent his time on 
his large patch, not lecture him and have him waste 4 more hours into a 
useless excercise of splitting which only causes more possible 
regressions and the stupidity of adding hacks to be removed 5 minutes 
later by a 2nd patch. And once again, even though there have been other 
large patches, it is mine you attack. When someone gives you a bag of 10 
000$, do you complanin that you would like it in 10 small briefcases?

Best regards,
Alex Ionescu

[Steven Edwards]

--- Alex Ionescu [e-mail] wrote:
> Most people/places would be very proud that a coder spent his time on 
> his large patch, not lecture him and have him waste 4 more hours into a 
> useless excercise of splitting which only causes more possible 
> regressions and the stupidity of adding hacks to be removed 5 minutes 
> later by a 2nd patch. And once again, even though there have been other 
> large patches, it is mine you attack. When someone gives you a bag of 10 
> 000$, do you complanin that you would like it in 10 small briefcases?

Here is a suggestion. Next time you or anyone else has a patch that is in debate for whatever
reason, submit it to the mailing list so it can have pear review. I am sure in a 2000 to 3000 line
patch there are at least a few minor bugs and by submitting it to the list you are ending this
type of debate so a patch can stand on its merits.

Thanks
Steven

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com

[Alex Ionescu]

Casper Hornstrup wrote:
>

Tell you what, the 10K line number is inflated by svn dif's stupidity. 
3K of it is just removing rosrtl, which I can already do most of in 
trunk. 2-3 more K are new functions which I can also add right now, 
since they are unused. My patch has less then 2K lines of real code.

Best regards,
Alex Ionescu

[Hartmut Birr]

James Tabor wrote:
> Hi all,
> Bored again, decided to build ros on ros.
>
> Getting this,
>
> (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
> (KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
> (KERNEL32:lib/kernel32/mem/global.c:412) Memory Load: 11
> (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
> (subsys/csrss/api/handle.c:75) CsrGetObject returning invalid handle
>
> Things I noticed may have been fixed,
> Multi cmd problem, handle count going throw the roof and hangs.

Can you describe the bug more exactly?

Currently I've a problem on the smp build. I build ros on ros. I do only
start one console window. After some time the output does freeze in the
console window. The cursor doesn't blink. I can move the mouse. I had
the feeling, that the sequence of locking some objects are wrong. I've
add some debug prints. It is not a problem of waiting on spinlocks or
fastmutexs. It is also not a problem of waiting on the message queues
from win32k. Currently, I've no idea what is wrong.

- Hartmut