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

[Alex Ionescu]

Joseph Galbraith wrote:

> ion at 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