Re: [ros-dev] Re: [ros-diffs] [ion] 12774: Add more Critical Section Debug Data and fix two bugs.

[Alex Ionescu]

Joseph Galbraith wrote:

>   ion(a)svn.reactos.com wrote:
>
> >   +    DPRINT("Waiting on Critical Section:
> > %x\n", CriticalSection);
> >  +    if (CriticalSection->DebugInfo) 
> >  CriticalSection->DebugInfo->EntryCount++; 
>
>  I'm just reading the diffs here, so I could be way
>  off, but shouldn't this use an InterlockedIncrement...
>  otherwise you could get a context switch between
>  in the middle of your increment to another threading
>  attempting to lock the critical section.  It is just
>  debug information, but still... 
I suppose that makes sense... I'll look into it.

>  
>  Also, looking at the rest of the code, I notice that
>  spin count doesn't appear to be implemented. 
No, not as of now. It was based on WINE code which doesn't implement 
this yet (AFAIK). I have been optimizing it and adding more feature to 
make it stabler, so spin lock support for MP builds was on my list. 
Thanks for your patch... I'll take a look at it.

>  
>  I don't have a build environment even setup, so
>  take the attached patch with a grain of salt.
>
>  The changes in RtlpCreateCriticalSEctionSem()
>  are because the InterlockCompareExchangePointer()
>  should already have written the new event into
>  the data structure. 
Yeah, you're right, it was a bit useless since the call already sets the 
new event if sucesful. We still have to compare with the old event 
though, not with 0.

>  
>  Maybe on day I'll manage to get my build environment
>  setup again. 
Are you on win32? Do you need any help setting it up?

>  
>  - Joseph
>  
Best regards,
Alex Ionescu