Skip to content

Conversation

@ForNeVeR
Copy link
Collaborator

@ForNeVeR ForNeVeR commented Dec 19, 2020

It turns out that some of our code works under HandleProcessCorruptedStateExceptions attribute on .NET Framework. This attribute allows the process to proceed execution even in case of memory access violation, but will only execute catch and finally blocks (including the implicitly generated finally in case of a lock block) in methods directly marked with the attribute.

So, if ByteBufferAsyncProcessor.Put is called from such method, and then a corrupted state exception occurs inside of the lock block, the lock will be abandoned in a locked state. This could lead to deadlocks that are hard to detect.

Provided we don't want to change the behavior of all the external code right now, the safest action we could take is to guard the path leading to our logger with the same attribute. It will at least help to preserve the logger functionality even in case of corrupted state exception, which should be helpful.

@ForNeVeR ForNeVeR requested review from sergeyQx and ulex December 19, 2020 15:23
@ForNeVeR ForNeVeR force-pushed the net211.im/safe-logger branch from f3bb6e8 to 6be05b1 Compare December 19, 2020 15:26
…ions

It turns out that some of our code works under
[HandleProcessCorruptedStateExceptions] attribute on .NET Framework.
This attribute allows the process to proceed execution even in case of
memory access violation, but will only execute catch and finally blocks
in direct methods marked with the attribute.

So, if ByteBufferAsyncProcessor.Put is called from such method, and then
an exception occurs inside of the lock block, the lock will be abandoned
in a locked state. This could lead to deadlocks that are hard to detect.

Provided we don't want to change the behavior of all the external code
right now, the safest action we could take is to guard the path leading
to our logger with the same attribute. It will at least help to preserve
the logger functionality even in case of corrupted state exception,
which should be helpful.
@ForNeVeR ForNeVeR force-pushed the net211.im/safe-logger branch from 6be05b1 to 55cf06c Compare December 19, 2020 15:30
@ForNeVeR ForNeVeR merged commit 996a3a2 into master Dec 20, 2020
@ForNeVeR ForNeVeR deleted the net211.im/safe-logger branch December 20, 2020 07:05


#if !NET35
[System.Runtime.ExceptionServices.HandleProcessCorruptedStateExceptions] // to force myLock to be unlocked even in case of corrupted state exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have also added logging of the exception

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, a good idea! See #190.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants