Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visual Studio crashes when opening files #36

Open
hiptopjones opened this issue Feb 15, 2024 · 11 comments
Open

Visual Studio crashes when opening files #36

hiptopjones opened this issue Feb 15, 2024 · 11 comments

Comments

@hiptopjones
Copy link

Installed product versions

  • Visual Studio:
    Microsoft Visual Studio Community 2022 (64-bit) - Current
    Version 17.8.7
  • This extension: 2.6

Description

Visual Studio crashes when opening files in project. This happens 100% for me in the below project.

Application: devenv.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.NullReferenceException
at CollapseComments.CollapseCommand+d__6.MoveNext()
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at Microsoft.VisualStudio.Threading.JoinableTaskFactory+SingleExecuteProtector.TryExecute()
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
at System.Windows.Threading.DispatcherOperation.InvokeImpl()
at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(System.Object)
at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
at MS.Internal.CulturePreservingExecutionContext.Run(MS.Internal.CulturePreservingExecutionContext, System.Threading.ContextCallback, System.Object)
at System.Windows.Threading.DispatcherOperation.Invoke()
at System.Windows.Threading.Dispatcher.ProcessQueue()
at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
at MS.Win32.HwndWrapper.WndProc(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
at MS.Win32.HwndSubclass.DispatcherCallbackOperation(System.Object)
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(System.Windows.Threading.DispatcherPriority, System.TimeSpan, System.Delegate, System.Object, Int32)
at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr, Int32, IntPtr, IntPtr)

Steps to recreate

  1. Clone https://github.com/tryAGI/LangChain
  2. Open the sln
  3. Open a couple of files, maybe navigate to definition (F12)

Current behavior

Visual Studio freezes and then crashes

Expected behavior

Not crashing

@mrlacey
Copy link
Owner

mrlacey commented Feb 15, 2024

Thanks for the awesome repro. 😄
Looking at this now....

@mrlacey
Copy link
Owner

mrlacey commented Feb 15, 2024

It's an exception being thrown in the error handling 🤦‍♂️

@mrlacey
Copy link
Owner

mrlacey commented Feb 15, 2024

v2.7 (just released to the marketplace) should stop the crash!

I'll follow up with future investigation as to why VS is now causing something internal to fail when it has worked without issue for 5+ years.

@mrlacey mrlacey self-assigned this Feb 15, 2024
@ryanmolden
Copy link

ryanmolden commented Feb 24, 2024

This is the number 17 crash in our internal crash data for VS 17.10 preview, so awesome to see it is fixed. What changed internally in VS that you mentioned in your last comment?

@mrlacey
Copy link
Owner

mrlacey commented Feb 24, 2024

This is the number 17 crash in our internal crash data for VS 17.10 preview, so awesome to see it is fixed. What changed internally in VS that you mentioned in your last comment?

Is that the number 17 issue is crashes all coming from only this extension, or it just so happens that lots of things are causing this exception, including this extension?


I'm not entirely sure what's changed as I haven't been able to make a proper, consistent repro.

The line that was failing was trying to create a SnapshotSpan of the whole TextView.
e.g.

new SnapshotSpan(this.viewHost.TextView.TextSnapshot, 0, this.viewHost.TextView.TextSnapshot.Length)

This had been working for years without issue but is now failing inconsistently.
I guess this is due to an internal change around the order of events happening when a document is loaded, or things are now happening asynchronously when previously they were happening synchronously.

The issue first reported here was when the extension was configured to collapse all comment regions when a document is opened. Sometimes it works fine and sometimes the exception is thrown. It's not dependent on the file being open or anything else in the project/solution or the extension. It's that the extension is trying to identify all the regions in a document at the same time that other things are happening (adding other adorners, adjusting line spacing, etc.) when a document is opened.

@ryanmolden
Copy link

ryanmolden commented Feb 26, 2024

Number 17 all up across all crashes in VS, extensions and product crashes, and its just from this extensions as Watson (the internal crash handling infrastructure) sorts things based on 'blame' and blame is formed off frames in the crash callstack, specifically the one it chooses is at faul, which is CollapseComments here. So all crash counts in that 'bucket' come from CollapseComments.

Definitely things have changed with async open/save. Is the code above running on the UI thread? What triggers it to happen?

@mrlacey
Copy link
Owner

mrlacey commented Feb 26, 2024

This is triggered by the OnBeforeDocumentWindowShow event from the RunningDocument events https://github.com/mrlacey/CollapseComments/blob/main/src/MyRunningDocTableEvents.cs#L66

The DTE executes a command the does run on the UI thread because it needs to access the ServiceManager and it uses the TextView to get the OutliningManger to find the regions to collapse.

@ryanmolden
Copy link

ryanmolden commented Feb 26, 2024

I pinged some folks doing work on the asyn document open/save stuff to see if this may be a regression.

NOTE: this is not doing what you think:

 // Offload to a background thread
this.package.JoinableTaskFactory.Run(async () =>
{
    await Task.Yield(); // get off the caller's callstack.
    await Task.Delay(200); // Give the document time to load as command won't work until outlining has loaded.
    this.dte.ExecuteCommand("CollapseComments.Collapse");
});

Specifically, command execution is inherently UI thread bound due to how we route commands and how many people have plugged in UI encumbered items as their command handlers. What this line ACTUALLY does is cause COM RPC to synchronously invoke BACK to the UI thread to execute your command.

If you wanted this to be actually async and the bits your command invokes supports this, don't use DTE, just invoke into your command handling code directly from the b/g thread (potentially after delaying, though delaying for 200 ms is arbitrary, better to drive ultimate execution off some more reliable mechanism to indicate 'outlining has loaded').

@mrlacey
Copy link
Owner

mrlacey commented Feb 26, 2024

I know the command is executed on the UI thread, the above was because it's running in something inherently synchronous, I get off the UI thread to delay to give the document a bit more time to load.

It's hacky but has worked for years (until this issue). 🤷‍♂️

I couldn't find a way to know for sure that the outlining has loaded (there are more delays elsewhere while waiting for it because its availability is dependent on factors that are based on the size & complexity of the document.)

@ryanmolden
Copy link

Oh, I see, sorry, thought you were moving to the threadpool thread, but I think the above yields will capture and resume on the UI thread so no COM RPC.

An editor dev said (in an internal mail) they were going to suggest a better way, it has to do with exporting an ITextViewCreationListener which will be invoked by the editor when the text view is ready for interaction. I will let him reply with the specifics as it isn't my area.

@mrlacey
Copy link
Owner

mrlacey commented Nov 17, 2024

Oh, I see, sorry, thought you were moving to the threadpool thread, but I think the above yields will capture and resume on the UI thread so no COM RPC.

An editor dev said (in an internal mail) they were going to suggest a better way, it has to do with exporting an ITextViewCreationListener which will be invoked by the editor when the text view is ready for interaction. I will let him reply with the specifics as it isn't my area.

@ryanmolden who's the "editor dev"? Can you share any more of what they said?

@mrlacey mrlacey removed their assignment Dec 6, 2024
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

No branches or pull requests

3 participants