5

I continue working on my TCP sockets, as mentioned this morning.

I have discovered that the amount of sockets keeps growing, every time I send a message over such a socket. This simply means that I am constantly creating and opening new sockets instead of re-using the already opened ones.

In my application, I have a repository of connections, and I only need to use one of them. But I'm afraid that I'm constantly creating new ones. This is the way it is done:

public static class Repository
{
  ...
  public static Conn GetByName(string name, ...)
  {
    return result = 
      context.Set<Conn>().Where(o => o.Name == name).FirstOrDefault();
  }
}

...
Conn Connection_For_Message = Repository.GetByName(request.ConnectionName, ...);

According to me:

  1. The fact that the Repository is declared static causes one single object to be created internally.
  2. As the GetByName() method performs a FirstOrDefault() of a Where() method, this is giving the already existing object, and it does not create a new one.

Now I have the impression that 1. is correct, but 2. is wrong, in the sense that Connection Connection_For_Message always creates a new instance of an object.

Is my impression correct and if yes, how can I solve this? I was thinking of altering Repository from a static class into a singleton, but I don't find an ISingleton or something in System.Reflection, or am I looking the completely wrong direction?

Edit: Hereby a part of the constructor of the Conn class:

public Conn(...)
{
    ...
    TcpConnection = new TcpConnection(...); // in here a 
    ...
}

As far as the TcpConnection class is concerned:

public class TcpConnection : IDisposable
{
    public Socket _socket;
    ...

Every time I send a message, I pass by the GetByName() method and my call stack looks as follows:

  My_Application.dll!TcpConnection.TcpConnection(...) Line 39   C#
> My_Application.dll!Conn.Conn(...) Line 32 C#
  [External Code]   
  My_Application.dll!ConnectionRepository.GetByName(string name, ...) Line 71   C#

The line, marked with > is the constructor, which proves indeed that GetByName() is calling the Conn constructor indeed. (Oh, and why is there "External code" between the GetByName() method and the constructor?)

Edit again: I have managed showing the external code, hereby the callstack with "Show External Code" checked:

>  My_Application.dll!TcpConnection.TcpConnection(...) Line 39  C#
   My_Application.dll!Conn.Conn(...) Line 32    C#
   [Native to Managed Transition]   
   [Managed to Native Transition]   
   System.Private.CoreLib.dll!System.Reflection.RuntimeConstructorInfo.Invoke(System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, object[] parameters, System.Globalization.CultureInfo culture)    C#
   System.Private.CoreLib.dll!System.RuntimeType.CreateInstanceImpl(System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder binder, object[] args, System.Globalization.CultureInfo culture)   C#
   System.Private.CoreLib.dll!System.Activator.CreateInstance(System.Type type, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder binder, object[] args, System.Globalization.CultureInfo culture, object[] activationAttributes)    C#
   Castle.Core.dll!Castle.DynamicProxy.ProxyGenerator.CreateClassProxyInstance(System.Type proxyType, System.Collections.Generic.List<object> proxyArguments, System.Type classToProxy, object[] constructorArguments)  C#
   [Lightweight Function]   
   Microsoft.EntityFrameworkCore.Relational.dll!Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable<Conn>.Enumerator.MoveNext()   C#
   System.Linq.dll!System.Linq.Enumerable.TryGetSingle<Conn>(System.Collections.Generic.IEnumerable<Conn> source, out bool found)   C#
   [Lightweight Function]   
   System.Linq.Queryable.dll!System.Linq.Queryable.FirstOrDefault<Conn>(System.Linq.IQueryable<Conn> source)    C#
   My_Application.dll!ConnectionRepository.GetByName(string name, ...) Line 71  C#
Ram
  • 131
  • 8
Dominique
  • 16,450
  • 15
  • 56
  • 112
  • 3
    "The fact that the Repository is declared static causes one single object to be created internally." No, it means there are *no* instances of `Repository`, and can't be. – Jon Skeet Dec 15 '22 at 14:59
  • 3
    But there's nothing in `GetByName` that obviously creates new instances of `Conn`. We don't know what `context.Set` does though - maybe *that* call creates new instances... – Jon Skeet Dec 15 '22 at 15:00
  • 1
    why don't you use your debugger, put a breakpoint on the line that creates sockets, and then see whether it keeps getting to that line over and over, or just once? – user253751 Dec 15 '22 at 15:04
  • I'm afraid your impression is wrong, `static` does nearly nothing. And I don't understand, does this question have anything do with `Reflection`? – shingo Dec 15 '22 at 15:08
  • It is true that the `GetByName()` passes via the constructor of the `Conn`, I have checked this using the debugger, I'll edit my question immediately. – Dominique Dec 15 '22 at 15:09
  • @shingo: I have a very limited knowledge of `Reflection`: I thought finding the `ISingleton` interface in there, but that was a bad hunch. – Dominique Dec 15 '22 at 15:10
  • You are querying database with Entity Framework to get Conn. Database of course cannot store such thing as TCP connection. Every time you do the query, new instance of Conn is returned, although it represents the same database row. Creating TCP Client in constructor of a class which represents database entry is not a good idea. – Evk Dec 15 '22 at 17:48
  • @Evk: thanks, that's a very useful remark. I'll need to re-think the whole idea of how to handle my TCP sockets in my application. – Dominique Dec 16 '22 at 07:09
  • @Evk: I've been doing some thinking: you state that launching a query in Entity Framework causes every time a new object to be created. However, while verifying this part of the call stack, I don't see references to Entity Framework (only `System.LinQ`), so I'm wondering if creating a new list of `Conn_with_socket` objects wouldn't cause the same issue while querying? – Dominique Dec 22 '22 at 08:16
  • The callstack shown in your question does contain reference to Entity Framework. Also it does not _always_ return new instance, it _might_ return cached instance, but that doesn't change anything related to your situation. Just use separate `ConcurrentDictionary` dictionary to manage your connections, or something similar (but first check how to use `ConcurrentDictionary` correctly, or use normal `Dictionary` and use locks). – Evk Dec 22 '22 at 08:33
  • @Evk: no need being so shy :-) I have replaced my `List` by `ConcurentDictionary` and launching `TryGetValue()` does not launch the constructor anymore, so this is working. Please rewrite your comment as an answer, I'll accept, upvote and reward it, your reputation will take a jump of 225 points :-) – Dominique Dec 22 '22 at 09:58

2 Answers2

6

You are querying database with Entity Framework to get Conn (as we can see from callstack). Database of course cannot store such thing as TCP connection. Every time you do the query, new instance of Conn is returned (technically it can be returned from cache but it doesn't change the situation), although it represents the same database row. Creating TCP Client in constructor of a class which represents database entry is not a good idea.

Instead you can use ConcurrentDictionary<string, TcpConnection> to manage your connections in a thread-safe way.

Evk
  • 98,527
  • 8
  • 141
  • 191
0

With lambada you can look for similar variables inside an object for a hashed variable or PK(primary key) Should that PK be the same skip creating this way we ensure that we're not creating new copies of an existing object with set PK.

   Var oldOjbect = context.object.firstordefault(o => o.pk == input.pk )
if(oldobject == null)
{ oldobject = new object { object.name = input.name}

  context.object.add(oldobject)
  context.SaveChanges();

seems like TcpConnection = new TcpConnection(...) your always creating new connections? Why not look for previous connection? With lambada search for tcpconnection's PK

Rojhat
  • 79
  • 7