Remove empty try's #13493
Remove empty try's #13493
Conversation
| // Windows NT 3.5 and later the timer resolution is approximately 10ms, | ||
| // for Windows NT 3.1 it is approximately 16ms, and for Windows 95 and 98 | ||
| // it is approximately 55ms. | ||
| // Windows NT 3.5 and later the timer resolution is approximately 10ms. |
There was a problem hiding this comment.
LOL, you fixed it while I was searching for that PR :)
There was a problem hiding this comment.
Yes, I dug up exactly that PR :)
|
@danmosemsft why are we removing the empty trys? Isn't the thread abort protection still needed in those regions? Funceval in VS uses thread abort to abort evaluation of functions that took too long time to execute and the changes you've made would seem to lead to leaks in case the abort happens in the region that were protected by the finally blocks with empty try before. |
|
My understanding was thread abort was no longer supported. There was a jit change recently that elided empty trys. Sounds like I misunderstood, or corefx is different? |
|
Thread abort in .NET Core is used for the limited func eval case only. The runtime and framework are not built and tested to be fully reliable in the presence of thread aborts. |
|
Was that broken by 119b04c ? |
danmoseley
added
the
* NO MERGE *
|
From the jit side, finally cloning also has a similar effect -- finally clauses are cloned to streamline the non-exception path performance and the clones are not reported as finallys to the runtime (see #8551). |
|
@AndyAyersMS can you clarify, do you mean that 119b04c means that empty try clauses are never seen by the runtime anyway? |
|
Not quite, just that empty try removal and finally cloning have similar impact on the way that the code originally within the finally clause is reported to the runtime. Finally cloning is more general and has more widespread impact and also came first, so it is the main culprit in the behavior change. |
|
On .Net Core, when the jit optimizes the code, then protection against thread abort is no longer guaranteed for code in finally clauses. Whether the try is empty or not empty does not matter. If the jit is not optimizing the code, what happens to code in finally clauses during thread abort is up to the runtime. |
|
I do not see a problem with removing them. There are hundreds of places that are not robust against thread abort. Having a few more won't make a difference. We are not building .NET Core to be fully reliable in the presence of thread abort. |
* Remove empty try's * Remove some dead comments * more Signed-off-by: dotnet-bot <[email protected]>
* Remove empty try's * Remove some dead comments * more Signed-off-by: dotnet-bot <[email protected]>
Just remove cases where we still had empty try's.
This isn't very interesting, but what is the status of CER's in CoreCLR? What does
RuntimeHelpers.PrepareConstrainedRegions();do? Is there cleanup possible?