Fix LSP Crash in Code Fences

[dandeandean]

Description

This PR fixes the crash described in #461.

The root cause of the problem was that the global state that tracks whether or not a line is in a code fence was not reset after each pass over the file. This behavior can be shown in the test-wrap.md file.

This would cause the currentCodeBlockStart to not always point to the fence.
Sometimes it would point to a line with len < 3, which resulted in a out of bounds read in lines[currentCodeBlockStart][:3] == line[:3].

Another undefined state caused by this is where lineIndex < currentCodeBlockStart.

Testing:

I manually tested by attaching the lsp to nvim 0v0.10.4 on the following files:
test-note.md
test-fence.md
test-wrap.md

[dandeandean]

@tjex @Rahlir I am still investigating the root cause of this issue. In the mean time could you take a look at this? I think it may be worth closing this out so there's no crash. Let me know

[tjex]

Great, thanks for this!! There's been a lot of activity here all of sudden, and I'm a little down on free time at the moment. So just a heads up that I may take a bit to get around to this.

[tjex]

So the test-note.md is causing the crash for me as expected.

Do you understand why the crash is happening?

I feel like it would be great to just hit the nail on the head with this from the start, instead of a hot fix.
Because this will likely become an "out of sight out of mind" situation. And then some time down the track we'll forget the context of the code you've provided, and that it should in fact be removed in place of a more robust solution.

[dandeandean]

@tjex That's a fair point. I haven't been working on this, but I can pick up on this again & try to find a more robust solution. I was being lazy & trying to get away with just fixing the crashing conditions.

It's strange that it's still crashing for you... how did you test it?

[tjex]

Ah sorry. I meant that I tested the functionality of the test-note to recreate a crash. And that worked.

If you wanted to do so that would be awesome. I can help along the way. But sporadically... Still getting to grips with the new job and balancing a masters thesis on the side.

But this bug is a high priority for me.

[dandeandean]

@tjex I've been looking this week trying to find out what causes these crashing conditions, but no dice. The problematic function is isLineWithinCodeBlock, which is called by DocumentLinks. DocumentLinks is called in line 596 in the server (which is a goroutine). We can see all of this in the stack trace.

We also call this DocumentLinks in handler.TextDocumentDocumentLink. I suspect our issue comes from that function calling DocumentLinks unexpectedly & messing up the expected state of the global variables that we keep track of fence information. It may be worth redesigning how this isLineWithinCodeBlocks works so it doesn't have side-effects.

Do you have any insight into when this handler.TextDocumentDocumentLink is called? I can't seem to find any documentation on glsp16.

[dandeandean]

Hey @tjex, after some more investigation (and a whole bunch of print debugging), I am pretty sure the fix was way more simple than I initially though. See the updated the description & the test files.

After compiling & attaching this version of the LSP, I haven't had any crashes.
If you could independently verify, that'd be great, but I think this is ready to merge.

[tjex]

Ooooooh dayum. That's a nice turn of events! Will check it out after dinner / tomorrow.
Thank you!!! This is an awesome result (if it does indeed do the trick haha). Thank you for your persistence!

[tjex]

All working on my end. Well done and thanks again!
Really happy about this 🥳