netbios，exception，and multithread

[Wudel]

Dear ADS Library Team,

I hope this message finds you well. I have been experimenting with the ADS library and have encountered a few issues that I would greatly appreciate your guidance on.

1. NetBIOS Protocol Requirement: It seems that the platform using this library requires the NetBIOS protocol to establish a connection; otherwise, the connection might be terminated by the remote PLC (such as TwinCAT). In a Linux environment, this implies the need for Samba. Could you confirm if this is indeed necessary?
2. Handling SIGPIPE Signals: During testing, I noticed that when the socket is broken, a SIGPIPE signal may be thrown, causing the program to crash. Would it be advisable to use signal(SIGPIPE, SIG_IGN) to ignore this signal? Or are there other recommended approaches to handle this situation?

3. ADS/AdsLib/standalone/AmsRouter.cpp

   Line 82 in 20d4a67

   throw e;

   
   Exception Handling in AmsRouter.cpp: While reviewing the code, I observed that the exception is thrown using throw e instead of simply throw. It seems that using throw would be preferable as it preserves the original exception type, allowing it to be caught as an adsException or runtime_error. Could you please give some advice on this?
4. multiThread: It appears that when multiple adsDevice instances are created using same remote ip and amsid, they share a single AmsConnection, which uses one TCP socket. The differentiation between these devices is based on their local ports, although these are not like traditional TCP ports. When I arrange one adsDevice per thread in multithreads, simultaneous requests from different devices seem to interfere with each other, possibly due to the shared sendto function. Can you confirm whether they will indeed affect each other?
   

   ADS/AdsLib/AdsDevice.h

   Line 85 in 20d4a67

   long ReadWriteReqEx2(uint32_t indexGroup,

   
   

   ADS/AdsLib/AdsDevice.cpp

   Line 201 in 20d4a67

   return AdsSyncReadWriteReqEx2(GetLocalPort(),

   
   

   ADS/AdsLib/standalone/AdsLib.cpp

   Line 106 in 20d4a67

   long AdsSyncReadDeviceInfoReqEx(long port, const AmsAddr* pAddr, char* devName, AdsVersion* version)

   
   

   ADS/AdsLib/standalone/AdsLib.cpp

   Line 255 in 20d4a67

   return GetRouter().AdsRequest(request);

   
   

   ADS/AdsLib/standalone/AmsRouter.cpp

   Line 204 in 20d4a67

   return ads->AdsRequest(request, ports[request.port - Router::PORT_BASE].tmms);

   
   

   ADS/AdsLib/standalone/AmsConnection.cpp

   Line 141 in 20d4a67

   AmsResponse* response = Write(request, srcAddr);

   
   

   ADS/AdsLib/standalone/AmsConnection.cpp

   Line 126 in 20d4a67

   if (request.frame.size() != socket.write(request.frame)) {

   
   

   ADS/AdsLib/Sockets.cpp

   Line 224 in 20d4a67

   const int status = sendto(m_Socket, buffer, bufferLength, 0, m_DestAddr, m_DestAddrLen);

[pbruenn]

Dear ADS Library Team,

I hope this message finds you well. I have been experimenting with the ADS library and have encountered a few issues that I would greatly appreciate your guidance on.

1. NetBIOS Protocol Requirement: It seems that the platform using this library requires the NetBIOS protocol to establish a connection; otherwise, the connection might be terminated by the remote PLC (such as TwinCAT). In a Linux environment, this implies the need for Samba. Could you confirm if this is indeed necessary?

I don't understand why NetBIOS should be required? You either have to use IP addresses or any kind of name resolution, but DNS should be just fine.

2. Handling SIGPIPE Signals: During testing, I noticed that when the socket is broken, a SIGPIPE signal may be thrown, causing the program to crash. Would it be advisable to use signal(SIGPIPE, SIG_IGN) to ignore this signal? Or are there other recommended approaches to handle this situation?

I have no better approach for this.

3. https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/standalone/AmsRouter.cpp#L82
   
   Exception Handling in AmsRouter.cpp: While reviewing the code, I observed that the exception is thrown using throw e instead of simply throw. It seems that using throw would be preferable as it preserves the original exception type, allowing it to be caught as an adsException or runtime_error. Could you please give some advice on this?

you are probably right. This should be a rethrow instead of copy and throw.

4. multiThread: It appears that when multiple adsDevice instances are created using same remote ip and amsid, they share a single AmsConnection, which uses one TCP socket. The differentiation between these devices is based on their local ports, although these are not like traditional TCP ports. When I arrange one adsDevice per thread in multithreads, simultaneous requests from different devices seem to interfere with each other, possibly due to the shared sendto function. Can you confirm whether they will indeed affect each other?

Yes, this is fundamental ADS design. TwinCAT will only accept a single TCP connection from the same IP. #201 (comment) #49 (comment)
#72 (comment)

   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/AdsDevice.h#L85
   
   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/AdsDevice.cpp#L201
   
   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/standalone/AdsLib.cpp#L106
   
   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/standalone/AdsLib.cpp#L255
   
   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/standalone/AmsRouter.cpp#L204
   
   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/standalone/AmsConnection.cpp#L141
   
   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/standalone/AmsConnection.cpp#L126
   
   https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/Sockets.cpp#L224

[Wudel]

#86 (comment)
Thank you very much. As mentioned in the link, regarding the fourth question: if we use different AdsDevice instances (each with a different port) in different threads, each AdsDevice will depend on the same socket connection. Should we use a mutex or another synchronization method to prevent these AdsDevice instances from sending adsRequest simultaneously?

[Wudel]

20241129_095256.mp4

For the first question, I am confused about how NetBIOS influences the program. In fact, there are no references to NetBIOS in the source code of the "main" program. NetBIOS runs as a service system and has no direct relationship with the "main" program. This makes the situation quite puzzling.

I tried to capture the network traffic using Wireshark and found that the PLC conducts a name query via NBNS once when we attempt to connect to the PLC. This may be a design feature of the PLC and seems to have no relationship with the ADS library. The remote PLC is running TwinCAT 2, with the CPU being CX1100-0002 and the Ethernet module being CX1020-0011. The errorcode is 1861. I hope this information will be helpful to others.

[pbruenn]

#86 (comment) Thank you very much. As mentioned in the link, regarding the fourth question: if we use different AdsDevice instances (each with a different port) in different threads, each AdsDevice will depend on the same socket connection. Should we use a mutex or another synchronization method to prevent these AdsDevice instances from sending adsRequest simultaneously?

No, it should be safe to use AdsDevices in parallel without external synchronization. This is our testcase for this: https://github.com/Beckhoff/ADS/blob/master/AdsLibOOITest/main.cpp#L497-L510