NeuronRobotics / nrjavaserial

A Java Serial Port system. This is a fork of the RXTX project that uses in jar loading of the native code.
Other
344 stars 142 forks source link

Only 63 COM Ports can be opened concurrently on Windows #235

Open BenjaminLetz opened 1 year ago

BenjaminLetz commented 1 year ago

I have more than 64 COM Ports configured in my Windows System. Windows 10 supports 4096 COM Ports.

After opening 63 Serialports with SerialPort serialPort = (SerialPort) commPortIdentifier.open("NRSerialTest", 0); my application just "waits" at the opening of the 64th. I suspect that only 63 SerialPorts can be opened at tha same time, but I do not get why.

I tried looking in the code, but did not find any restrictions.

Here is a small test application I wrote:

` public static void main(String[] args) {

    ArrayList<CommPortIdentifier> commPortIdentifiers= new ArrayList<>();
    ArrayList<SerialPort> serialPorts= new ArrayList<>();
    for(String s:NRSerialPort.getAvailableSerialPorts())
    {
        System.out.println(s);
        try {
            CommPortIdentifier commPortIdentifier = CommPortIdentifier.getPortIdentifier(s);
            commPortIdentifiers.add(commPortIdentifier);
            SerialPort serialPort = (SerialPort) commPortIdentifier.open("NRSerialTest", 0);
            serialPorts.add(serialPort);
            //serialPort.close();
        } catch (NoSuchPortException e) {
            System.out.println("Not a Port: " + s);
        } catch (PortInUseException e) {
            System.out.println("Port in Use: " + s);
        }
    }
}`

When I add the serialPort.close(); line, the program finishes with all ports being opened once.

Some Information abput my system: Windows 10 22H2 Build 19045 Java 11.0.2

fwolter commented 1 year ago

A quick look into the code reveals this line: https://github.com/NeuronRobotics/nrjavaserial/blob/e897e3978aba971c43c77ef72cf1e8593376e3b8/src/main/c/src/SerialImp.c#L4779

You could try to increase it.

MrDOS commented 1 year ago

Thanks, @fwolter, beat me to it.

You can also try putting JNA 4 on your classpath. If it's available at runtime, port detection uses it to enumerate serial ports from the registry:

https://github.com/NeuronRobotics/nrjavaserial/blob/e897e3978aba971c43c77ef72cf1e8593376e3b8/src/main/java/gnu/io/RXTXCommDriver.java#L537

That should bypass the upper bound set in the native code.

When investigating https://github.com/NeuronRobotics/nrjavaserial/issues/196, the only documented limit to the number of serial ports on Windows that I could find was 256. On the driver side, the maximum number of serial ports is given by a constant that isn't specifically published but does seem to be 4,096. Out of curiosity, are you aware of any official documentation to support the 4,096-port limit in Windows 10?

BenjaminLetz commented 1 year ago

Hi,

thanks for the quick answers! so I found the for loop as well and tried to change it to 65 or 4096 and I had the same result.

I also think that the jna should be present. At least it is in my maven dependencies. But I will try to debug the native code and see where it might have a problem.

Regarding the official info about the 4096 ports. I did not find where I got that from, but I think it was somerwhere in the Windows Registry. If I find it I will post it here.

BenjaminLetz commented 1 year ago

I think I may have found the Problem.

In the SerialImpl.c in the method initialise_event_info_struct the constant FD_SETSIZE is used to see if a port can be opened. This constant seems to be set to 64 on Windows by standard.

I tried just adding 1 to it there and I was successful opening another port.

Now when I add the following lines to the SerialImpl.h i can open all my ports.

#ifdef FD_SETSIZE 
#undef FD_SETSIZE 
#define FD_SETSIZE 4096
#endif

I have to say, that I am not a C and Windows expert so I do not know if this has any side effects in the application. For my usecase it seems to work.

Regarding the maximum COM ports on Windows if found this site where it says: "There is no limit to the number of COM ports that Windows supports."

https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/external-naming-of-com-ports

MrDOS commented 1 year ago

In the SerialImpl.c in the method initialise_event_info_struct the constant FD_SETSIZE is used to see if a port can be opened.

Good find!

#define FD_SETSIZE 4096

Interesting. This would definitely blow up on POSIX systems where the value of FD_SETSIZE is tightly tied to the implementation of fd_set, used by select(2) to wait for activity on the serial port. On Windows, though, the serial_select function (src/main/c/src/windows/termios.c:3138) emulates select(2) with WaitCommEvent/WaitForSingleObject, which don't care about file descriptors.

I think the safest approach here is to probably define the platform-dependent maximum port number; on POSIX, this can be based on FD_SETSIZE, and on Windows, this can be arbitrarily high:

diff --git a/src/main/c/include/SerialImp.h b/src/main/c/include/SerialImp.h
index 329140e..c9dc443 100644
--- a/src/main/c/include/SerialImp.h
+++ b/src/main/c/include/SerialImp.h
@@ -281,12 +281,16 @@ Trent
 #  define DEVICEDIR "//./"
 #  define LOCKDIR ""
 #  define LOCKFILEPREFIX ""
+// Windows has no strict limit to the number of open serial ports, but we need
+// to define something.
+#  define PORT_MAX 4096
 #  define OPEN serial_open
 #  define CLOSE serial_close
 #  define WRITE serial_write
 #  define READ serial_read
 #  define SELECT serial_select
 #else /* use the system calls for Unix */
+#  define PORT_MAX FD_SETSIZE
 #  define OPEN open
 #  define CLOSE close
 #  define WRITE write
diff --git a/src/main/c/src/SerialImp.c b/src/main/c/src/SerialImp.c
index 5ea7404..fcd6af4 100644
--- a/src/main/c/src/SerialImp.c
+++ b/src/main/c/src/SerialImp.c
@@ -4195,15 +4195,15 @@ int initialise_event_info_struct( struct event_info_struct *eis )
    }
 end:
    FD_ZERO( &eis->rfds );
-   if (eis->fd < FD_SETSIZE && eis->fd > 0) {
+   if (eis->fd < PORT_MAX && eis->fd > 0) {
        FD_SET( eis->fd, &eis->rfds );
        eis->tv_sleep.tv_sec = 0;
        eis->tv_sleep.tv_usec = 100 * 1000;
        eis->initialised = 1;
        return( 1 );
-   } else if(eis->fd >= FD_SETSIZE ){
+   } else if(eis->fd >= PORT_MAX ){
        // you can reduce this limitation only with migration to epool or something like that.
-       report_error("initialise_event_info_struct: eis->fd >= FD_SETSIZE!\n");
+       report_error("initialise_event_info_struct: eis->fd >= PORT_MAX!\n");
    } else if(eis->fd <= 0 ){
        // you can reduce this limitation only with migration to epool or something like that.
        report_error("initialise_event_info_struct: eis->fd <= 0!\n");

I'm glad you have an application-specific solution that works for you for now. I'll package my diff up as a PR and get that up in a bit.

MrDOS commented 1 year ago

Hm, actually, that still might cause problems on Windows, because initialise_event_info_struct and read_byte_array are still going to call FD_SET with a file descriptor larger than FD_SETSIZE, which is undefined behaviour:

An _fdset is a fixed size buffer. Executing FD_CLR() or FD_SET() with a value of fd that is negative or is equal to or larger than FD_SETSIZE will result in undefined behavior. Moreover, POSIX requires fd to be a valid file descriptor.

Mind you, serial_select then goes on to totally ignore the the contents of the readfds/writefds sets and just uses ndfs - 1 as the single file descriptor to monitor. So we can probably safely work around that by redefining FD_SET macro as a no-op on Windows.

BenjaminLetz commented 1 year ago

I am testing it with a seperate PORT_MAX constant and it seems to work as well.

But I get that you don't want to run into undefined behaviour.

MrDOS commented 1 year ago

Thanks for the feedback!

Btw, out of curiosity, where are you getting all of these serial ports from? Are these virtual/network serial ports, or do you have that many actual hardware serial ports? (Just wondering if there's a convenient way for me also to get 64+ ports for local reproduction of this issue.)

BenjaminLetz commented 1 year ago

Our actual usecase is using network provided COM Ports via COM servers from Digi, but I am simulating the COM ports with a tool called com0com.

With that tool you can simulate COM Pairs that communicate with each other so you can communicate with your own System.

https://com0com.sourceforge.net/ https://sourceforge.net/projects/com0com/

I hope this helps you and thanks for your help :)

BenjaminLetz commented 1 year ago

Do you want me to open a pull request with these changes or will you do this?

MrDOS commented 1 year ago

Thanks for offering – I want to play with the FD_SET definition to avoid hitting UB, so I'll open a PR.