davidfowl / BedrockFramework

High performance, low level networking APIs for building custom servers and clients.
MIT License
1.05k stars 153 forks source link

System.IO.Pipelines.StreamPipeWriter heisenbugs #111

Closed RoccoDevs closed 3 years ago

RoccoDevs commented 3 years ago

I'm currently working on a server using bedrock. And I've been hitting a weird bug where I get one of the two following errors:

Object reference not set to an instance of an object. at System.IO.Pipelines.StreamPipeWriter.FlushAsyncInternal(Boolean writeToS
tream, CancellationToken cancellationToken)
         at Bedrock.Framework.Protocols.ProtocolWriter.WriteAsync[TWriteMessage](IMessageWriter`1 writer, TWriteMessage protocolMessage, CancellationToken cancellationToken)
         at TheDesolatedTunnels.RelayServer.Core.Services.MessageSender.TrySendAsync(SixtyNineSendibleMessage requestMessage, CancellationToken cancellationToken) in D:\Source\the-desolated-tunnels-relay
\TerminalGame.RelayServer\TheDesolatedTunnels.RelayServer.Core\Services\MessageSender.cs:line 78
         at TheDesolatedTunnels.RelayServer.Core.ConnectionHandlers.SixtyNineProtocolHandler.OnConnectedAsync(ConnectionContext connection) in D:\Source\the-desolated-tunnels-relay\TerminalGame.RelayServ
er\TheDesolatedTunnels.RelayServer.Core\ConnectionHandlers\SixtyNineProtocolHandler.cs:line 83

or:

Argument null exception: Value cannot be null. (Parameter 'array')    System.Buffers.TlsOverPerCoreLockedStacksArrayPool`1.Return(T[] array
, Boolean clearArray)
         at System.IO.Pipelines.BufferSegment.ResetMemory()
         at System.IO.Pipelines.StreamPipeWriter.FlushAsyncInternal(Boolean writeToStream, CancellationToken cancellationToken)
         at Bedrock.Framework.Protocols.ProtocolWriter.WriteAsync[TWriteMessage](IMessageWriter`1 writer, TWriteMessage protocolMessage, CancellationToken cancellationToken)
         at TheDesolatedTunnels.RelayServer.Core.Services.MessageSender.TrySendAsync(SixtyNineSendibleMessage requestMessage, CancellationToken cancellationToken) in D:\Source\the-desolated-tunnels-r
elay\TerminalGame.RelayServer\TheDesolatedTunnels.RelayServer.Core\Services\MessageSender.cs:line 78
         at TheDesolatedTunnels.RelayServer.Core.ConnectionHandlers.SixtyNineProtocolHandler.OnConnectedAsync(ConnectionContext connection) in D:\Source\the-desolated-tunnels-relay\TerminalGame.Relay
Server\TheDesolatedTunnels.RelayServer.Core\ConnectionHandlers\SixtyNineProtocolHandler.cs:line 83

I've tried a lot, but I'm out of luck I guess. But it really seems to be a bug inside of bedrock and not my code. Hence why I'm submitting this as an issue. Hope someone knows what's up! I have the source code on github. Feel free to take a peek

davidfowl commented 3 years ago

When does it happen? Are you accessing the connection after it's been disposed?

RoccoDevs commented 3 years ago

No the connections aren't disposed yet, so that's not the problem. I log whenever they get disposed and I can verify that way.

The server works like a relay or network switch. Some client A wants to message client B. The server mediates this a bit like signalR. And this error usually shows up more often when connections first come in. But also happens after that when traffic gets busy.

I committed a full copy of the code to github. If you would like to take a peek you can find it here: https://github.com/tomesendam/RelayServer.Benchmarking/blob/main/src/TheDesolatedTunnels.RelayServer.Core/Services/MessageSender.cs

It should be noted that this code is quite experimental at the moment.

jzabroski commented 3 years ago

I have to say, there is a certain harmony in having BedrockFramework next to TheDesolatedTunnels

jzabroski commented 3 years ago

@tomesendam Did you try loading the current src for Bedrock into your project and adding Conditional breakpoints right where it bombs out. You should get good stack details there.

RoccoDevs commented 3 years ago

@tomesendam Did you try loading the current src for Bedrock into your project and adding Conditional breakpoints right where it bombs out. You should get good stack details there.

I did not, I'll try that in a bit!

The naming is a pure coincidence 😂

RoccoDevs commented 3 years ago

I seem to have "fixed" the bug. I surrounded the code in TrySendAsync with a SemaphoreSlim. It seems the SemaphoreSlim within bedrock's WriteAsync wasn't enough? But I'm just learning about all of this. I honestly don't know why it fixed it. I assumed bedrock's semaphore would be enough. @davidfowl If you have time I would like to know why if you do know!

davidfowl commented 3 years ago

Odd that the semaphore in the protocol writer isn't enough.

RoccoDevs commented 3 years ago

Odd that the semaphore in the protocol writer isn't enough.

Yes it is. You have no idea what's going wrong either?