Azure / DotNetty

DotNetty project – a port of netty, event-driven asynchronous network application framework
Other
4.09k stars 977 forks source link

System.ArgumentException: Buffer is empty is still unsolved in 0.5.0 #422

Closed slmgong closed 6 years ago

slmgong commented 6 years ago

centos 7.2.1511 mono Version 5.12.0.301 dotnetty Version 0.5.0 msbuild my app in .netframwork 4.6.2 the bug sometimes arise and sometimes not, but in windows running successful everytimes

Unhandled Exception: System.ArgumentException: Buffer is empty Parameter name: buffers at System.Net.Sockets.SocketAsyncResult.CheckIfThrowDelayedException () [0x00014] in <3379537a28364286889e0f112c2e834e>:0 at System.Net.Sockets.Socket.EndSend (System.IAsyncResult asyncResult, System.Net.Sockets.SocketError& errorCode) [0x00055] in <3379537a28364286889e0f112c2e834e>:0 at System.Net.Sockets.Socket.EndSend (System.IAsyncResult asyncResult) [0x00000] in <3379537a28364286889e0f112c2e834e>:0 at System.Net.Sockets.Socket+<>c.<.cctor>b__309_12 (System.IAsyncResult ares) [0x0002c] in <3379537a28364286889e0f112c2e834e>:0 at System.Net.Sockets.SocketAsyncResult+<>c.b__27_0 (System.Object state) [0x0000b] in <3379537a28364286889e0f112c2e834e>:0 at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem () [0x00008] in <21e3540b4da8479f80a213d385157d24>:0 at System.Threading.ThreadPoolWorkQueue.Dispatch () [0x00074] in <21e3540b4da8479f80a213d385157d24>:0 at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback () [0x00000] in <21e3540b4da8479f80a213d385157d24>:0 [ERROR] FATAL UNHANDLED EXCEPTION: System.ArgumentException: Buffer is empty

yyjdelete commented 6 years ago

https://github.com/Azure/DotNetty/blob/e67df79/src/DotNetty.Transport/Channels/Sockets/TcpSocketChannel.cs#L250-L333

Seems sharedBufferList is not copy when !done && writtenBytes == 0 now as it did in #280

yyjdelete commented 6 years ago

@vivid216 Can you check if #423 works for you?

slmgong commented 6 years ago

I think @yyjdelete are right, while localWrittenBytes ==0 the bufferList is reference sharedBufferList object , let's checkout the #423 and do a test.

https://github.com/Azure/DotNetty/issues/422#issuecomment-418980232

slmgong commented 6 years ago

I test with #423 code , and the "Buffer is empty" Exception seems been solved, but caught another "data-corruption" exception sometimes. is it should be break in any case while this.IncompleteWrite(true, asyncOperation) be executed both in TcpSocketChannel and AbstractSocketByteChannel ? @yyjdelete

caozhiyuan commented 6 years ago

https://github.com/Azure/DotNetty/pull/336 @yyjdelete maybe this pull lose it( ArraySegment[] copiedBuffers = sharedBufferList.ToArray(); )?
this work ok on linux( .net core 2.0.3 above )

caozhiyuan commented 6 years ago

@yyjdelete .net core SocketAsyncEventArgs set BufferList not different from mono , that case this. https://github.com/dotnet/corefx/blob/master/src/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs#L161

yyjdelete commented 6 years ago

@caozhiyuan But seems the logic is only include for netcoreapp >= 2.0. Any idea for the data-corruption? Is is something like you reported before?

https://github.com/dotnet/corefx/blob/v1.1.9/src/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs#L128 https://referencesource.microsoft.com/#System/net/System/Net/Sockets/Socket.cs,8737 https://github.com/mono/mono/blob/master/mcs/class/System/System.Net.Sockets/SocketAsyncEventArgs.cs#L68

caozhiyuan commented 6 years ago

1、netcoreapp 2.0.3 below has socket send bug in linux (sendasync is ok ), since 2.0.3 fix it because linux socket diff from windows socket send. @yyjdelete 2、.net core and .net framework socket before send will copy bufferlist , but mono will not.

caozhiyuan commented 6 years ago

data-corruption it need debug on mono , use strace like this strace -f -e sendmsg dotnet TestTCP.dll @yyjdelete

caozhiyuan commented 6 years ago

@yyjdelete data corruption this https://github.com/Azure/DotNetty/issues/286

caozhiyuan commented 6 years ago

debug check localWrittenBytes value , in linux maybe not send all buffer , this will return send lit bytes. if localWrittenBytes always 0 , maybe mono socket bug @yyjdelete

yyjdelete commented 6 years ago

Seems mono works well, but there is an issue in ChannelOutboundBuffer.GetSharedBufferList(int,long)/RemoveBytes(long).

That DotNetty use ArraySegment<byte> as nioBuf instead of something like IByteBuffer in netty which has an changeable readerIndex, so the cache of nioBuf can not be used for the first entry.

https://github.com/Azure/DotNetty/blob/4bd2621c/src/DotNetty.Transport/Channels/ChannelOutboundBuffer.cs#L395-L401

caozhiyuan commented 6 years ago

@yyjdelete this.flushedEntry.Buffers = null ? if partial writen , the left will be Remove on sendasync callback.

yyjdelete commented 6 years ago

@caozhiyuan See #423

caozhiyuan commented 6 years ago

@yyjdelete
i can't understand this.flushedEntry.Buffer = new ArraySegment(); this.flushedEntry.Buffers = null;

yyjdelete commented 6 years ago

@caozhiyuan Otherwise, when call DoWrite the next time, GetSharedBufferList still use the old cached nioBuffer with old readerIndex, and send some bytes for the second time. Note ClearNioBuffers() only clear the cache for ChannelOutboundBuffer but not for single Entry.

Example(don't follow maxCount to make it simpler): "Hello World1"(nocache),"Hello World2"(nocache),"Hello World3"(nocache) Send(18 bytes)->"Hello World1","Hello "(Would block, and "Hello World3") Remove->"World2"(cached with "Hello World2"),"Hello World3"(nocache) Send(12 bytes)->"Hello World2"(from stale nio buffer cache with readerIndex unchanged) Remove->"World3"(nocache) Send(6 bytes)->"World3"

Finally client get "Hello World1","Hello Hello World 2","World3" which is wrong.

caozhiyuan commented 6 years ago

@yyjdelete you try it on .net coreapp2.0.3 in linux . input.RemoveBytes(writtenBytes); has changed readerIndex . and sendasync will send the left all buffer .
is mono sendasync will not send all buffer? i konw ClearNioBuffers clear list and buffer is pooled https://github.com/Azure/DotNetty/blob/master/src/DotNetty.Transport/Channels/Sockets/TcpSocketChannel.cs#L311 if writtenBytes > 0 input.RemoveBytes(writtenBytes) and IncompleteWrite(true, asyncOperation) will sendasync the left all buffer . i can't understand Send(12 bytes)->"Hello World2"(from stale nio buffer cache with readerIndex unchanged)

yyjdelete commented 6 years ago

@caozhiyuan Not tested, but infact there is no code to change the offset of the nioBuf's cache. According to the issue you file to corefx, it may also happen on netcoreapp in linux.

But in windows(dotnet/netfx), WouldBlock always return 0. So no partly writen will happen. even try Send(new byte[655360000],...), the first two call get Success, 655360000. And the third call get WouldBlock, 0.


Updated

@caozhiyuan Maybe I have some mistake it's only needed in my local branch(try sync WriteSpinCount logic with netty 4.1) to change readerIndex, the rest buffer is send with PrepareWriteOperation in master.

Will test latter to see the really callstack. Seems it's really not happen after reset the buffer, and I capture the package like descributed before https://github.com/Azure/DotNetty/issues/422#issuecomment-419708422

But maybe still needed here with maxCount limited, to remove no needed buffer.(very rare case) https://github.com/Azure/DotNetty/blob/4bd2621c/src/DotNetty.Transport/Channels/ChannelOutboundBuffer.cs#L414


The thing wrong is that Mono may do partly Send with SendAsync, so still need to reset the buffer cache.

Send Success, 1728/3757, 15
Send WouldBlock, 0/2029, 14
RemoveBytes , 1728
IncompleteWrite , True
FinishWrite , 1152/2029
RemoveBytes , 1152
caozhiyuan commented 6 years ago

this code ? https://github.com/Azure/DotNetty/blob/master/src/DotNetty.Transport/Channels/Sockets/TcpSocketChannel.cs#L305 and when sent input.RemoveBytes(writtenBytes) @yyjdelete

caozhiyuan commented 6 years ago

@yyjdelete in linux socket has 1024 send bufferlist count limit. netcoreapp 2.0.5 runtime seems fix it. but this code is same with java. if this (The thing wrong is that Mono may do partly Send with SendAsync, so still need to reset the buffer cache.) , sendasync callback must check sentbytes and loop to sendasync all buffer. now platform diff is big for developer.

this (https://github.com/Azure/DotNetty/blob/4bd2621c/src/DotNetty.Transport/Channels/ChannelOutboundBuffer.cs#L414) seems has problem , i check it it seems my code always IoBufferCount==1 , no CompositeByteBuffer

slmgong commented 6 years ago

` // Did not write all buffers completely.

                    if (this.IncompleteWrite(true, asyncOperation))

                    {

                        break;

                    }`

i remove if condition this.IncompleteWrite(true, asyncOperation); break; looks like work well ?

caozhiyuan commented 6 years ago

@yyjdelete use CompositeByteBuffer , maxCount 1024 has problem . now i understand this.flushedEntry.Buffer = new ArraySegment(); this.flushedEntry.Buffers = null;

yyjdelete commented 6 years ago

https://github.com/mono/mono/blob/master/mcs/class/System/System.Net.Sockets/Socket.cs#L1910 @vivid216 In Mono, SendAsync always return true, so no difference between them.

@caozhiyuan There is check for e.Buffer(see the comment, and note is always use non-blocking), but not for e.BufferList in Mono. https://github.com/mono/mono/blob/master/mcs/class/System/System.Net.Sockets/Socket.cs#L1982 https://github.com/mono/mono/blob/master/mcs/class/System/System.Net.Sockets/Socket.cs#L2021

netcoreapp seems only work well with e.BufferList in 2.0.0+, the test expected all datas should be send, but undocumented. https://github.com/dotnet/corefx/pull/19791

caozhiyuan commented 6 years ago

below netcoreapp 2.0.0 , has some other socket bug , such as 1024 limit , socket send wouldblock always return 0 . i don't check if below netcoreapp 2.0.0 has fix it or not. we use netcoreapp 2.0.3 + online in linux . @yyjdelete

caozhiyuan commented 6 years ago

now the problem is mono socket sendasync not send all buffer ? @yyjdelete

yyjdelete commented 6 years ago

@caozhiyuan Will report it latter.

And maybe is ok to add an ChannelOption instead of the hard code 1024, but it's out of the issue.

yyjdelete commented 6 years ago

See mono/mono#10533


@caozhiyuan You are right. Updated, thanks.

caozhiyuan commented 6 years ago

@yyjdelete
sould
receivedChecksum.Add(recvBuffer, 0, received); if (sendTask.IsCompleted && bytesReceived == sendTask.Result) { break; }

caozhiyuan commented 6 years ago

@yyjdelete mono has 1024 bufferlist limit too. it will raise Message too long , sdk don't make some compatible

slmgong commented 6 years ago

update 0.6.0 looks been solved