davidfowl / BedrockFramework

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

Is ClientBuilder.Build supposed to be Threadsafe ? #72

Closed Ninds closed 4 years ago

Ninds commented 4 years ago

@drawaes As per your suggestion I replaced Pipelines.Sockets.Unofficial with Bedrock for client side connections. Also I cached the instance of ClientBuilder for re-use. However it appears that the Build method is not threadsafe. If ClientBuilder clientBuilder = new ClientBuilder(new ServiceCollection().BuildServiceProvider()).UseSockets();

and I execute :

await clientBuilder.Build().ConnectAsync(new IPEndPoint(IPAddress.Loopback, Port), default);

from 20 threads, I have the following often :

System.ArgumentException : Destination array was not long enough. Check the destination index, length, and the array's lower bounds. (Parameter 'destinationArray') Stack Trace: Array.Copy(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable) List'1.CopyTo(T[] array, Int32 arrayIndex) EnumerableHelpers.ToArray[T](IEnumerable'1 source, Int32& length) Buffer'1.ctor(IEnumerable'1 source) ReverseIterator'1.MoveNext() ConnectionBuilder.Build() ClientBuilder.Build() line 55

If on the other hand I use :

clientBuilder.Build().ConnectAsync(new IPEndPoint(IPAddress.Loopback, Port), default).Result; no exception. Am I missing something ?

Drawaes commented 4 years ago

I wouldn't cache the builder but the actual client. So then you just call ConnectAsync each time on the one cached client (not client builder). That way you can take advantage of things like connection pooling etc

davidfowl commented 4 years ago

That’s right. You build the client once (which itself is a factory) and that then can be used to create more connections.

Ninds commented 4 years ago

Thanks Guys! It's what I figured out before going to lunch! Bloody obvious with hindsight when you see that there is an effort to reproduce on the client side the pattern seen on the server side.