What's wrong with this code, part 6 - the answers

re: What's wrong with this code, part 6 - the answers

Pavel Lebedinsky — Thu, 09 Sep 2004 21:26:00 GMT

> Consider a folder with an ACL that allows
> the user to create files, but not write
> to them.

Note however that users can always change permissions on any files they created, regardless of what the DACL says.

Your review folder example will work only if you apply additional restrictions at the share level, to prevent users from changing the DACL.

> EnterCriticalSection should be protected
> with a try/catch since it can throw

No it shouldn't. On XP and later, Enter/LeaveCS never raise exceptions. On Win2K and before, they can raise, but you can't recover from this, even if you use try/except (try/catch should only be used for C+ exceptions; in Whidbey catch(...) will not even catch SEH exceptions by default).

The reason you can't recover is because on Win2K the critical section could be left in a corrupted state when Enter or LeaveCriticalSection throws.

The bottom line is that you should never wrap Enter/LeaveCriticalSection in a try/except. On XP and later, it's not necessary, and on Win2K it doesn't really help anyway.

If you need critical sections to work reliably in out-of-memory conditions on Win2K your only option is to preallocate the event using InitializeCriticalSectionAndSpinCount.

re: What's wrong with this code, part 6 - the answers

Larry Osterman — Thu, 09 Sep 2004 21:41:00 GMT

A good point Pavel.

And I meant try/except, not try/catch, I'll update the text.

re: What's wrong with this code, part 6 - the answers

Skywing — Fri, 10 Sep 2004 21:55:00 GMT

I never said it raises on Windows XP or later. In those cases it uses a system wide KeyedEvent (new kernel object) to avoid having to allocate memory for each critical section having it's own wait semaphore.

Note that MSDN *RECOMMENDS* that you use SEH to catch EnterCriticalSection exceptions ( http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/entercriticalsection.asp ).

How do you arrive at the conclusion that the critical section will be put into an invalid state if an exception is thrown? RtlLeaveCriticalSection throws an exception if NtSetEvent fails, which can only happen if the structure itself is damaged or somebody messed with the event handle (which are obviously nonrecoverable, but unrelated to running out of memory). I didn't write the critsec code, but I don't *think* the lockcount being incremented an extra time will cause any fatal problems -- perhaps a lock cmpxchg with 0 when it's not really necessary, but that oughtn't break anything.

re: What's wrong with this code, part 6 - the answers

Pavel Lebedinsky — Fri, 10 Sep 2004 22:56:00 GMT

> How do you arrive at the conclusion that
> the critical section will be put into an
> invalid state if an exception is thrown?

Here are some quotes from Neill Clift (he is the guy who fixed critical sections for XP):

"It was really impossible to handle exceptions generated by EnterCriticalSection and LeaveCriticalSection because they left the CS in a state you couldn't recover from."

http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&selm=utHsttTmBHA.2432%40tkmsftngp07

"I considered it impossible to code to Enter/Leave raising. To make matters worse the internal structure of the lock was damaged by the raise such that it might be unusable after the event."

http://groups.google.com/groups?selm=3fe1d456%241%40news.microsoft.com&output=gplain

The fact that LeaveCriticalSection can also raise in low memory situations (on Win2K) seems unintuitive at first but it's true - there's some obscure scenario where allocation of the event has to happen in LeaveCriticalSection instead of EnterCriticalSection.

re: What's wrong with this code, part 6 - the answers

Skywing — Fri, 10 Sep 2004 23:09:00 GMT

Ah, okay. In that case, somebody should update the MSDN documentation to not recommend something which doesn't work. :)

Updating Application Data

Don Kiely's Technical Blatherings — Sun, 26 Sep 2004 03:24:00 GMT

Updating Application Data