tagging @jinujoseph

If this can happen in "regular" usage like that, then it seems odd that a Debug.Assert is the right thing either. Is it possible to have something a bit more precise, or at least in the disconnect case do something to prevent the assert from firing?

killing OOP is not regular thing. we show info bar and ask users to shutdown VS. we just don't want to hard crash VS.

in normal case, users shouldn't see assert. if they do, it is a bug like the one that had test flaky.

forgot to tag @jasonmalinowski

@heejaechang I guess asserting in "rare but handled" error conditions still isn't valid for an assert. My concern is simply the next time somebody sees this assert, they might think it's a bug and re-file it over and over again.

@jasonmalinowski I think with the info bar saying VS is unstable. I dont think people will see it not expected.

I guess my personal assumption would be that the assert is what caused the gold bar, and thus there's a problem. ;-)

@jasonmalinowski I see... hmmm.. but I can't think of any other way except removing it completely. I can change wording.

If this can happen in regular usage like that, then it seems odd that a Debug.Assert is the right thing either. Is it possible to have something a bit more precise, or at least in the disconnect case do something to prevent the assert from firing?

Agreed. Asserts are for things that should not happen, unless we have a bug in our own code. A dependent process going down can definitely happen.

it would be like us trying to do IO, and hten asserting that it can't fail.

I think assert is still useful. and it is only for debug bits. and I updated wording. so. I think it is fine

retest ubuntu_16_mono_debug_prtest please

ping? @dpoeschl @mavasani @ivanbasov can you take a look?

@jaredpar where should I see log for test failure - https://ci.dot.net/job/dotnet_roslyn/job/master/job/windows_debug_spanish_unit32_prtest/382/

retest windows_debug_spanish_unit32_prtest please

@heejaechang dump file of the hang is here

https://ci.dot.net/job/dotnet_roslyn/job/master/job/windows_debug_spanish_unit32_prtest/382/artifact/Binaries/Debug/Logs/

well, we used to crash VS when external process goes away. so I dont think it is worse than that.

I don't think we should count improving such a low bar as "good". 😄

if it is crashing then, I agree, but this is debug assert. we used to crash on external process get killed by users since it was exceptional case you guys insisted. so not sure why sudden change in mind and now it becomes normal thing that can happen so there should be no assert.

in normal situation, the assert will not happen. (unless there is a bug)
in exceptional situation where we used to crash, it might assert in debug (things to be perfectly align it to happen)

we no longer crash since we had enough feedbacks saying don't crash in all cost regardless it puts VS unstable state or not.

and I am not sure why this is a low bar. only concern I see is if info bar shows up and assert shows up people might open an issue we need to close as by design.

@dotnet/roslyn-analysis @dotnet/roslyn-ide @jinujoseph

ping? any other opinion? I want this in to master to remove chance of VS crashing. want to leave the assert since it have caught a bug.

and I am not sure why this is a low bar. only concern I see is if info bar shows up and assert shows up people might open an issue we need to close as by design.

I'm sorry but it doesn't make sense to me that an assert failure is by design. Assert should only fail if there's a bug in your program.

@Neme12 it will be. OOP get killed without users explicitly doing so is a bug. and it will assert.

question is users explicitly killing OOP is something we consider as expected or not. that question, long time ago, we already answered as it is exceptional case, exceptional enough for us to hard code to explicitly crash VS when that happens.

Right now, we removed crashing due to completely different reason. but that is not because we changed our mind on whether we consider it as expected situation or not. if we changed our mind, we should recover from it like the IO failure example above rather than showing info bar asking users to restart VS.

..

now, why we not crashing. it is due to completely unrelated topic,

basically, we are having enough feedbacks from users saying just don't crash. we used to crash VS just because we can't colorize a keyword. or because outlining had 1 position off or any smallest issue we have. we just crashed VS.

our reasoning was that it didn't do what we expected, so we can't guarantee state of VS 100%. so we can't trust VS anymore so crash.

now we are moving away from that model to don't crash VS as much as we can model.

now VS shows various info bar (yellow bar thing below menu) based on what feature is failing instead of crash. some we even just silently ignore and send non fatal watson. some we show info bar saying something went wrong, do you want to ignore this kind of issue from now on or turn off the feature. or some we say "VS got bad, please close and reopen VS"

for this particular case, we show "VS god bad. please close and reopen VS". since this is exceptional case, we do not try to recover from it. just simply moved from traditional hard crash to soft crash.