Removed errant proc.kill()

[rben01]

In #2098 I tried killing the process. It turns out that doing this before waiting leads to out of order IO, leading to IO errors when Bincode tries to deserialize. It also turns out that killing isn't even necessary; waiting is sufficient.

So this PR removes that kill line, getting rid of the IO errors introduced in #2098.

Also Added finish to benchmark_diagnostics as well (should have been there all along). Thought: do we want to just impl Drop for TestHarness? This might make the panicking situation tricky (generally, you shouldn't panic in drop), whereas explicitly calling harness.finish(), while possible to forget, makes handling panics just work.

[github-actions]

Bencher Report

Branch	lsp-server-fix
Testbed	ubuntu-latest

Click to view all benchmark results

Benchmark	Latency	nanoseconds (ns)
fibonacci 10	📈 view plot 🚷 view threshold	496,100.00
foldl arrays 50	📈 view plot 🚷 view threshold	1,724,900.00
foldl arrays 500	📈 view plot 🚷 view threshold	6,873,100.00
foldr strings 50	📈 view plot 🚷 view threshold	7,351,700.00
foldr strings 500	📈 view plot 🚷 view threshold	64,697,000.00
generate normal 250	📈 view plot 🚷 view threshold	50,633,000.00
generate normal 50	📈 view plot 🚷 view threshold	2,160,600.00
generate normal unchecked 1000	📈 view plot 🚷 view threshold	3,400,800.00
generate normal unchecked 200	📈 view plot 🚷 view threshold	782,600.00
pidigits 100	📈 view plot 🚷 view threshold	3,268,800.00
pipe normal 20	📈 view plot 🚷 view threshold	1,510,600.00
pipe normal 200	📈 view plot 🚷 view threshold	10,795,000.00
product 30	📈 view plot 🚷 view threshold	839,350.00
scalar 10	📈 view plot 🚷 view threshold	1,541,900.00
sum 30	📈 view plot 🚷 view threshold	843,590.00

🐰 View full continuous benchmarking report in Bencher

[jneem]

Thought: do we want to just impl Drop for TestHarness?

I don't have a strong opinion. Since this is test code, accidentally aborting due to a panic in drop isn't a big deal. On the other hand, we don't construct TestHarness that often, so remembering to call finish is probably not too hard. So either way is really ok with me.

[rben01]

Since I did in fact forget to call finish once, it's probably best to impl Drop for Server, bundling shutting down and waiting into it, so that they can't be forgotten. I don't expect the errors to actually ever be thrown, so panicking should be rare. And shutdown was never actually used anywhere, so I just moved it right into drop.