fix(network,mte): bind test hosts to an ephemeral port, not 25777 #5399
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -184,7 +184,6 @@ | |
| // Start the server. | ||
| serverChannelFuture = b.bind(); | ||
|
|
||
| if (config.getServerMOTD() != null) { | ||
| logger.info("Server MOTD is \"{}\"", config.getServerMOTD()); //NOPMD | ||
| } else { | ||
|
|
@@ -195,6 +194,9 @@ | |
| logger.info("Server started"); | ||
| } | ||
| serverChannelFuture.sync(); | ||
| // Local address is only reliably populated once bind() has completed - hence after sync(), | ||
| // not next to the bind() call above. Matters when port was 0 (see getBoundPort()). | ||
| logger.info("Started server on port {}", getBoundPort()); | ||
| nextNetworkTick = time.getRealTimeInMs(); | ||
| } catch (ChannelException e) { | ||
| if (e.getCause() instanceof BindException) { | ||
|
|
@@ -374,6 +376,14 @@ | |
| return mode; | ||
| } | ||
|
|
||
| @Override | ||
| public int getBoundPort() { | ||
| if (serverChannelFuture == null) { | ||
| return -1; | ||
| } | ||
| return ((InetSocketAddress) serverChannelFuture.channel().localAddress()).getPort(); | ||
| } | ||
|
Comment on lines
+379
to
+385
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
file="engine/src/main/java/org/terasology/engine/network/internal/NetworkSystemImpl.java"
printf '%s\n' '--- target methods and surrounding lifecycle code ---'
sed -n '150,220p;330,405p' "$file"
printf '%s\n' '--- declarations and all assignments/usages ---'
rg -n -C 3 'serverChannelFuture|int getBoundPort|void shutdown|shutdown\(' "$file"
printf '%s\n' '--- related interface contract and tests ---'
rg -n -C 4 'getBoundPort|bound port|serverChannelFuture|failed.bind|bind failure' engine/src/main/java engine/src/test 2>/dev/null || trueRepository: MovingBlocks/Terasology Length of output: 17753 🏁 Script executed: #!/bin/bash
set -eu
file="engine/src/main/java/org/terasology/engine/network/internal/NetworkSystemImpl.java"
printf '%s\n' '--- complete shutdown method ---'
sed -n '260,325p' "$file"
printf '%s\n' '--- imports and build dependency declarations for Netty ---'
sed -n '1,80p' "$file"
rg -n -i -C 3 'netty|io\.netty' --glob 'build.gradle*' --glob 'gradle.properties' --glob '*.gradle' --glob '*.toml' .
printf '%s\n' '--- tests for NetworkSystemImpl lifecycle ---'
fd -t f -i 'NetworkSystemImpl|NetworkSystem.*Test|.*Network.*Test' engine/src/test 2>/dev/null | head -80Repository: MovingBlocks/Terasology Length of output: 8330 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- allChannels registration and server-channel lifecycle ---'
rg -n -C 5 'allChannels|channelGroup|register.*Channel|add\(' engine/src/main/java/org/terasology/engine/network/internal
printf '%s\n' '--- Netty version and any vendored Channel implementation/contract ---'
rg -n -C 3 'interface Channel|localAddress\(\)|isOpen\(\)|class AbstractChannel' . --glob '*.java' --glob '*.md' --glob '*.kt' | head -160Repository: MovingBlocks/Terasology Length of output: 39855 🌐 Web query:
💡 Result: In Netty, calling shutdownGracefully on an EventLoopGroup effectively manages the lifecycle of all channels registered to that group [1][2][3]. When shutdownGracefully is invoked, the EventLoopGroup automatically closes all channels currently registered to it as part of the shutdown process [2][3]. Regarding your question about the localAddress, calling channel.close (which is performed automatically during the group shutdown) invalidates the channel's state, including its local and remote addresses [4]. While the Channel API's localAddress method is still accessible, it typically returns the address the channel was bound to until the underlying socket is fully closed and the internal state is cleared [4][5]. Once the channel is closed, it is no longer bound, and the localAddress method will correctly reflect that it is no longer bound (often returning null or an empty/invalid address depending on the specific implementation) [4][5][6]. In summary, you do not need to manually iterate over channels to close them or clear their addresses when shutting down the group; the EventLoopGroup performs this cleanup automatically [1][2][3]. If you need to ensure all operations are complete before proceeding, you should wait for the Future returned by shutdownGracefully to complete [1][7][3]. Citations:
🌐 Web query:
💡 Result: In Netty 4.1.77.Final, the relationship between NioServerSocketChannel, localAddress, close, and AbstractChannel/AbstractNioChannel follows the standard Netty transport hierarchy [1][2][3]. NioServerSocketChannel is a specific implementation of a server-side channel that uses NIO selectors to accept incoming connections [1]. It inherits from AbstractNioChannel, which in turn inherits from AbstractChannel [2][3]. Key technical details regarding these components: 1. Local Address: The local address (localAddress) of a NioServerSocketChannel is typically bound during the bind operation [4]. The underlying Java NIO ServerSocketChannel is used to manage this binding. You can retrieve the address using the localAddress method provided by the Channel interface [4]. 2. Close Operation: The close operation is defined in the Channel interface and implemented in AbstractChannel [2][4]. When close is invoked, AbstractChannel handles the coordination of closing the channel resources [2]. It ensures that the operation is executed within the EventLoop to maintain thread safety, handles the outbound buffer (flushing or failing pending messages), and initiates the doClose method [2]. 3. AbstractChannel and AbstractNioChannel: - AbstractChannel serves as the base class for all channel implementations, providing the foundational state management, lifecycle handling (like close), and pipeline integration [2]. - AbstractNioChannel extends AbstractChannel to add NIO-specific functionality, such as managing the SelectableChannel (the Java NIO primitive) and integrating with the Selector for non-blocking I/O operations [3]. When you call close on a NioServerSocketChannel, the AbstractChannel logic ensures the channel is properly closed and deregistered from the EventLoop, and subsequent events (like channelInactive) are fired as appropriate [2]. If you are encountering issues where close behavior seems unexpected (such as channelInactive being called twice), this is often related to the pipeline state or manual closure logic rather than a bug in the AbstractChannel close implementation itself [5][6]. Citations:
🌐 Web query:
💡 Result: In Netty 4.1.77.Final, the localAddress implementation in NioServerSocketChannel.java is straightforward. It overrides the superclass method to provide a type-safe return of InetSocketAddress [1][2]. The implementation follows this pattern: Citations:
Guard
🤖 Prompt for AI Agents |
||
|
|
||
| @Override | ||
| public Server getServer() { | ||
| return this.server; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: MovingBlocks/Terasology
Length of output: 7090
🏁 Script executed:
Repository: MovingBlocks/Terasology
Length of output: 36720
🏁 Script executed:
Repository: MovingBlocks/Terasology
Length of output: 7610
Do not reject a valid OS-assigned port.
NetworkSystem#hostpasses port0to Netty, andgetBoundPort()returns the socket's actual local port. The operating system can assign25777when it is available. RemoveAssertions.assertNotEquals(25777, boundPort)and keep the positive-port assertion.🤖 Prompt for AI Agents