native io transport as enum configurable #3
Conversation
native io transport as enum configurable also an option to detect the best automatically
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 112 files out of 219 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughReplaces three boolean native-transport flags with a single TransportType enum (default AUTO); SocketIOServer now probes and loads transports via reflection with NIO fallbacks; adds exported nativeio package; replaces PlatformDependent concurrent-map usages with ConcurrentHashMap; simplifies several tests and container readiness behaviors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Config as BasicConfiguration
participant Server as SocketIOServer
participant Probe as TransportProbe (reflection)
participant Netty as NettyBootstrap
Note over Server,Config: server startup -> determine transport
Server->>Config: getTransportType()
alt transport == AUTO
Server->>Probe: probe IO_URING
Probe-->>Server: available / not
alt not available
Server->>Probe: probe EPOLL
Probe-->>Server: available / not
alt not available
Server->>Probe: probe KQUEUE
Probe-->>Server: available / not
end
end
else explicit transport selected
Server->>Probe: check chosen transport availability
Probe-->>Server: available / not
end
Server->>Probe: load channel class & handler factory (reflection)
Probe-->>Server: channel class / handler or NIO fallback
Server->>Netty: configure bootstrap with selected channel & groups
Netty-->>Server: bind result (success/failure)
alt success
Server-->>Config: set running state
else failure
Server-->>Config: log error / set stopped
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
netty-socketio-spring-boot-starter/src/test/java/com/corundumstudio/socketio/test/spring/boot/starter/config/SocketIOOriginConfigurationTest.java (1)
75-75: Assertion correctly updated fortransportTypeThis assertion now matches the new
getTransportType()API and keeps the Spring Boot starter test aligned with core configuration defaults. Consider adding a separate test that sets a non-defaultnetty-socket-io.transport-typeproperty (e.g.,EPOLLorNIO) and verifies it flows through toConfigurationfor end-to-end coverage.netty-socketio-micronaut/src/test/java/com/corundumstudio/socketio/test/micronaut/config/SocketIOOriginConfigurationTest.java (1)
70-70: Micronaut test aligned withtransportTypeAPIThe new
getTransportType()equality check correctly reflects the enum-based configuration. As with the Spring Boot test, you might add a Micronaut test that overridesnetty-socket-io.transport-typeto a non-default value and asserts it reachesConfigurationto guard against future mapping regressions.netty-socketio-quarkus/netty-socketio-quarkus-runtime/src/main/java/com/corundumstudio/socketio/quarkus/recorder/NettySocketIOConfigRecorder.java (1)
77-77: Quarkus recorder correctly forwardstransportType, verify defaults/migrationWiring
configuration.setTransportType(nettySocketIOBasicConfigMapping.transportType())is the right way to bridge the Quarkus config mapping to the new enum-based core API. Please double‑check that:
NettySocketIOBasicConfigMapping.transportType()always returns a non-null value (defaulting toAUTO), and- Any previous Quarkus config keys for the old boolean flags are either still accepted and mapped, or clearly documented as a breaking change with migration guidance.
netty-socketio-core/src/main/java/com/corundumstudio/socketio/nativeio/TransportType.java (1)
1-9: Enum design looks good; consider central Javadoc for semanticsThe
TransportTypeenum cleanly models the supported transports and documents platform expectations inline. To make behavior easier to discover, consider adding enum-level Javadoc summarizing:
- The selection/fallback order used by
AUTO, and- How explicit choices behave when the native transport isn’t available (e.g., fallback to NIO).
This keeps the contract visible to API consumers without them having to inspect the server implementation.
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (2)
234-241: Include the bind failure cause in startAsync() loggingOn bind failure, the listener logs only the port, dropping the actual exception from
future.cause(). That makes diagnosing startup issues harder, especially around native transport availability and permissions.Consider including the cause in the log:
- return b.bind(addr).addListener((FutureListener<Void>) future -> { - if (future.isSuccess()) { - serverStatus.set(ServerStatus.STARTED); - log.info("SocketIO server started at port: {}", configCopy.getPort()); - } else { - serverStatus.set(ServerStatus.INIT); - log.error("SocketIO server start failed at port: {}!", configCopy.getPort()); - } - }); + return b.bind(addr).addListener((FutureListener<Void>) future -> { + if (future.isSuccess()) { + serverStatus.set(ServerStatus.STARTED); + log.info("SocketIO server started at port: {}", configCopy.getPort()); + } else { + serverStatus.set(ServerStatus.INIT); + log.error("SocketIO server start failed at port: {}!", configCopy.getPort(), future.cause()); + } + });
64-65: ServerStatus FSM is clear; consider how ignored start/stop requests should surface to callersUsing
AtomicReference<ServerStatus>with explicit states (INIT → STARTING → STARTED → STOPPING → INIT) makes lifecycle transitions clearer andisStarted()cheap and thread-safe.One behavioral point to double‑check: when
startAsync()orstop()are called in an invalid state, you log a warning and:
startAsync()returns a succeeded future that does nothing.stop()returns void after logging.If callers rely on the returned
Futureto notice that a start request was ignored, you may want to fail the future (or throw) instead of silently succeeding. If this matches previous behavior and your public contract, then the current approach is fine; just worth confirming.Also applies to: 161-163, 170-175, 336-350
netty-socketio-core/src/main/java/com/corundumstudio/socketio/BasicConfiguration.java (1)
19-20: TransportType core configuration looks good; be mindful of external API consumersThe new
transportTypefield with defaultTransportType.AUTO, copy‑constructor wiring, and simple getter/setter are all consistent with how other configuration properties are handled, and they align with the new enum‑driven transport selection inSocketIOServer.Given that this replaces previous native‑transport boolean flags, it’s worth double‑checking:
- That all framework integrations (Spring Boot, Micronaut, Quarkus) and internal call sites now exclusively use
getTransportType()/setTransportType(...).- That you’re comfortable with the public API break for any external code that previously used the boolean accessors (if this isn’t a major version bump, you might want to consider keeping deprecated bridge methods for one cycle).
Functionally, the new configuration surface looks sound.
Also applies to: 37-38, 76-80, 355-361
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/BasicConfiguration.java(4 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java(5 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/nativeio/TransportType.java(1 hunks)netty-socketio-core/src/main/java/module-info.java(1 hunks)netty-socketio-micronaut/src/test/java/com/corundumstudio/socketio/test/micronaut/config/SocketIOOriginConfigurationTest.java(1 hunks)netty-socketio-quarkus/netty-socketio-quarkus-runtime/src/main/java/com/corundumstudio/socketio/quarkus/config/NettySocketIOBasicConfigMapping.java(2 hunks)netty-socketio-quarkus/netty-socketio-quarkus-runtime/src/main/java/com/corundumstudio/socketio/quarkus/recorder/NettySocketIOConfigRecorder.java(1 hunks)netty-socketio-spring-boot-starter/src/test/java/com/corundumstudio/socketio/test/spring/boot/starter/config/SocketIOOriginConfigurationTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (25) / build
- GitHub Check: build (21) / build
- GitHub Check: build (17) / build
🔇 Additional comments (2)
netty-socketio-core/src/main/java/module-info.java (1)
15-15: Export ofnativeiopackage is appropriateExporting
com.corundumstudio.socketio.nativeiois consistent with makingTransportTypepart of the public API and allows frameworks/extensions to reference it without reflection hacks.netty-socketio-quarkus/netty-socketio-quarkus-runtime/src/main/java/com/corundumstudio/socketio/quarkus/config/NettySocketIOBasicConfigMapping.java (1)
26-27: TransportType mapping and docs look consistent with core behaviorThe Quarkus config now exposes
TransportTypewith a sensible default (AUTO) and Javadoc that matches the actual selection order and fallback behavior inSocketIOServer. The enum type and@WithDefault("AUTO")are aligned with SmallRye ConfigMapping expectations; no changes needed here.Also applies to: 70-95
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (1)
254-262: LGTM! Fallback helpers reduce duplication effectively.Both methods correctly encapsulate the fallback-to-NIO behavior and reduce code duplication across the transport selection switches.
Minor terminology nit:
fallback()logs "Falling back to NIO" whilefallbackFactory()logs "Falling back to NIOHandler." Consider using consistent terminology (e.g., both say "NIO" or both say "NIO transport") to avoid any confusion about whether these refer to the same fallback behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java(5 hunks)
🔇 Additional comments (3)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (3)
183-223: LGTM! Critical issue from previous review has been resolved.The explicit
case NIO:(lines 203-205) is now present and correctly setschannelClass = NioServerSocketChannel.class. This ensures symmetry with theinitGroups()IoHandler selection (lines 313-315) and guarantees that explicitly selectingTransportType.NIOforces pure NIO transport without falling through to native transport detection.The switch now properly handles all transport types with consistent fallback behavior when native transports are unavailable.
238-246: LGTM! Clean refactor to lambda syntax.The conversion to lambda-based
FutureListeneris correct and maintains the same state transition logic (serverStatusupdates) and logging behavior as the previous implementation.
293-329: LGTM! IoHandler selection correctly mirrors channel selection logic.The switch maintains perfect symmetry with the channel selection logic in
startAsync():
- Explicit transport types (IO_URING, EPOLL, KQUEUE, NIO) select the corresponding IoHandler with appropriate fallbacks
- AUTO mode applies the same priority detection (IO_URING → EPOLL → KQUEUE → NIO)
- The
case NIO:ensures explicit NIO selection usesNioIoHandler.newFactory(), maintaining consistency with the channel class selectionThis ensures the event loop groups and channel classes are always paired correctly.
not all nativeio packages will be as kqueue is unix based and iouring & epoll is linux based , so there is possiblity to get error while using the package directly, so we use reflection to check it which is error free and some test optimisations
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (1)
162-164: NIO case now explicitly handles forced NIO selection (previous issue resolved).The explicit
case NIO:in the channel selection switch addresses the critical issue flagged in the previous review. This ensures that when a user explicitly selects NIO, the server usesNioServerSocketChannelinstead of falling through to AUTO mode and potentially selecting a native transport.This now correctly mirrors the IoHandler selection logic at lines 272-274.
</review_comment_end>
🧹 Nitpick comments (4)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/RedissonStoreTest.java (1)
39-42: Container reuse for Redis looks good; verify test isolation with persistent stateEnabling
withReuse(true)is a nice speedup and consistent with the Hazelcast tests, but it also means the Redis container (and its data) can live across test runs. That’s fine as long as tests never depend on a completely empty Redis and either:
- clean up keys they touch, or
- use sufficiently unique key prefixes / DB indexes per test.
Might be worth double‑checking that
AbstractStoreTestor these tests don’t rely on Redis starting empty each time, otherwise you could see subtle cross‑test interference when reuse is actually enabled.netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/CustomizedHazelcastContainer.java (1)
20-75: Good move to Testcontainers readiness; consider tightening timeout and validating config mappingThe refactor to use
withExposedPorts(HAZELCAST_PORT)pluswaitingFor(Wait.forListeningPort())is a solid improvement over the previous manual sleeps/polling and should make tests more reliable and faster once the container is up.Two minor points to consider:
- The
Duration.ofSeconds(300)startup timeout is very generous; in most CI environments Hazelcast should be up much faster. You might want to reduce this (e.g. 60–120s) so genuinely stuck containers fail fast instead of hanging the suite for 5 minutes.- Since the container now relies on
hazelcast-test-config.xmlviawithClasspathResourceMapping(..., BindMode.READ_ONLY), it’s worth double‑checking that this resource is always on the test runtime classpath in all modules/IDE setups, or failures will be a bit opaque.Otherwise the lifecycle logging in
start()/stop()and the streamlinedconfigure()look clean.netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/HazelcastStoreTest.java (1)
37-40: Hazelcast container reuse is fine; ensure store/tests don’t rely on a fresh clusterUsing
withReuse(true)here matches the Redis test and will speed up Hazelcast-backed tests when reuse is enabled globally. Just be aware that:
- Maps and other Hazelcast data structures may persist across tests while the container is reused.
- Current tests should either clean up any used structures/keys or be written so that leftover data doesn’t affect assertions.
It might be worth a quick pass over the Hazelcast store tests (and any future ones) to ensure they remain isolated under a long‑lived container.
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (1)
122-127: Early return creates a temporary EventLoop (minor inefficiency in error path).When the server is not in the
INITstate, the method returns aSucceededFuture<>(new DefaultEventLoop(), null)(line 126). Creating aDefaultEventLoopjust for this future is slightly inefficient, though this only occurs in error cases.Consider using a cached or shared executor for the succeeded future, or document why a new
DefaultEventLoopis necessary here.</review_comment_end>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java(5 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/integration/AbstractSocketIOIntegrationTest.java(3 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/CustomizedHazelcastContainer.java(1 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/HazelcastStoreTest.java(1 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/RedissonStoreTest.java(1 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java(2 hunks)
🧰 Additional context used
🪛 ast-grep (0.40.0)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/integration/AbstractSocketIOIntegrationTest.java
[info] 141-141: "Detected use of a Java socket that is not encrypted. As a result, the
traffic could be read by an attacker intercepting the network traffic. Use
an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory'
instead."
Context: new ServerSocket(0)
Note: [CWE-319] Cleartext Transmission of Sensitive Information [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(unencrypted-socket-java)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (25) / build
- GitHub Check: build (17) / build
- GitHub Check: build (21) / build
🔇 Additional comments (8)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java (1)
242-246: Good refactoring using try-with-resources.The refactoring to use try-with-resources ensures the
ServerSocketis properly closed automatically, which is more concise and idiomatic than the previous explicit management.netty-socketio-core/src/test/java/com/corundumstudio/socketio/integration/AbstractSocketIOIntegrationTest.java (2)
141-143: Static analysis false positive: unencrypted socket is appropriate here.The static analysis tool flagged this for using an unencrypted socket. This is a false positive—the
ServerSocketis only used to obtain an available ephemeral port number, not for transmitting data. No security concern exists here.</review_comment_end>
193-198: Port validation relaxed but appears unnecessary.The port validation was relaxed from
<= 0to< 0(line 196), but this seems unnecessary. ThefindAvailablePort()method (line 183) returns a port > 0, and line 193 refreshes it from the server configuration. The port should never be exactly 0 at this point.However, this is a minor defensive check and the change is harmless.
</review_comment_end>
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (5)
136-177: Transport selection logic is comprehensive and correctly prioritized.The channel class selection properly handles all five transport types:
- Native transports (IO_URING, EPOLL, KQUEUE) check availability and fall back to NIO
- NIO is unconditional (no native dependencies)
- AUTO mode follows the documented priority: IO_URING → EPOLL → KQUEUE → NIO
The logic is consistent and matches the PR objectives.
</review_comment_end>
254-291: IoHandler selection correctly mirrors channel class selection.The
initGroups()switch structure parallels the channel selection logic, ensuring that theIoHandlerFactorymatches the chosenServerChanneltype. This consistency is critical for correct Netty operation.Each native transport case uses
createHandler()with reflection, while NIO uses the directNioIoHandler.newFactory(). The fallback behavior is consistent with channel selection.</review_comment_end>
207-226: Reflection-based loading is appropriate for optional native transports.The reflection approach allows the code to detect and load native transport classes dynamically without requiring them as compile-time dependencies. Key observations:
- Catching
Throwable(lines 212, 303) is correct for class loading, whereErrorsubtypes likeNoClassDefFoundErrorcan occur@SuppressWarnings("unchecked")is appropriately used for unavoidable casts after reflection- Methods are
private, limiting the scope of reflection usage- Fallback behavior is consistent and logged
[approve_code_challenges]
</review_comment_end>Also applies to: 293-306
395-430: Native transport detection uses proper two-step availability checks.The availability detection correctly combines two checks:
- Class presence check (
isClassPresent): Verifies the optional native transport dependency is on the classpath- Runtime availability check (
isNativeAvailable): Calls the transport'sisAvailable()method to verify the native library is loadedBoth steps catch
Throwableand returnfalseon failure, ensuring safe fallback behavior. The use ofSocketIOServer.class.getClassLoader()is appropriate for loading classes in the same module.</review_comment_end>
54-54: Minor code quality improvements throughout.Several small improvements enhance code quality:
- Line 54: Diamond operator for cleaner generics
- Lines 93-111: Simplified null checks for collections
- Line 130: Clearer log message formatting
- Line 391: Parameter renamed to
clazz(conventional forClass<?>parameters)These changes follow Java best practices.
</review_comment_end>
Also applies to: 93-111, 130-130, 391-393
...e/src/test/java/com/corundumstudio/socketio/integration/AbstractSocketIOIntegrationTest.java
Outdated
Show resolved
Hide resolved
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@sanjomo Fixed |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/RedissonPubSubStore.java (1)
76-84: Add null‑safety when removing registration IDs inunsubscribe
map.remove(name)can returnnullwhen there were no registrations for the givenPubSubTypeon this node, which will cause aNullPointerExceptionin the enhanced for loop. A simple guard avoids this:public void unsubscribe(PubSubType type) { String name = type.toString(); - Queue<Integer> regIds = map.remove(name); - RTopic topic = redissonSub.getTopic(name); - for (Integer id : regIds) { - topic.removeListener(id); - } + Queue<Integer> regIds = map.remove(name); + if (regIds == null || regIds.isEmpty()) { + return; + } + + RTopic topic = redissonSub.getTopic(name); + for (Integer id : regIds) { + topic.removeListener(id); + } }This keeps
unsubscribesafe even if called redundantly or out of order.
🧹 Nitpick comments (5)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java (1)
174-192: Consolidate duplicate null checks.The method checks
bodyForPost != nulltwice (lines 175 and 180). Since both checks guard related POST operations, consolidate them into a single conditional block for cleaner code structure.Apply this diff to consolidate the logic:
private static @NotNull HttpURLConnection getHttpURLConnection(String bodyForPost, HttpURLConnection http) throws IOException { if (bodyForPost != null) { http.setRequestMethod("POST"); // PUT is another valid option http.setDoOutput(true); - } - - if (bodyForPost != null) { byte[] out = bodyForPost.getBytes(StandardCharsets.UTF_8); http.setFixedLengthStreamingMode(out.length); http.setRequestProperty("Content-Type", "application/x-www-form-urlencoded; charset=UTF-8"); http.connect(); try (OutputStream os = http.getOutputStream()) { os.write(out); } } else { http.connect(); } return http; }netty-socketio-core/src/main/java/com/corundumstudio/socketio/BasicConfiguration.java (1)
21-22: Guard against nullTransportTypein the setterThe new
transportTypefield and copy-constructor wiring look good, butsetTransportTypeaccepts null, which would later causeswitch (configCopy.getTransportType())inSocketIOServerto throw aNullPointerException.Consider defensively rejecting or normalizing null here (e.g., defaulting to
AUTO):- public void setTransportType(TransportType transportType) { - this.transportType = transportType; - } + public void setTransportType(TransportType transportType) { + if (transportType == null) { + throw new IllegalArgumentException("transportType must not be null"); + } + this.transportType = transportType; + }Also applies to: 37-38, 76-80, 355-361
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (3)
60-65: Server state machine viaAtomicReferencelooks good; consider a lighter executor for the noop futureUsing
AtomicReference<ServerStatus>withcompareAndSetinstartAsyncandstopgives you a clear state machine and prevents concurrent start/stop races; the transitions and logging look consistent.For the invalid-start path, you currently create a new
DefaultEventLoopfor eachSucceededFuture. To avoid accumulating extra event loop instances, consider reusing a shared executor (e.g., a singleton event executor) instead ofnew DefaultEventLoop():- return new SucceededFuture<>(new DefaultEventLoop(), null); + return new SucceededFuture<>(io.netty.util.concurrent.GlobalEventExecutor.INSTANCE, null);(or a similar shared executor you already use in the project).
Also applies to: 126-135, 317-320
142-205: TransportType-based channel & IoHandler selection mostly symmetric; consider tightening fallback couplingThe new TransportType-based selection for both:
- server channel (
channelClassvialoadChannelClass/fallback), and- event loops (
IoHandlerFactoryviacreateHandler/fallbackFactoryininitGroups)is a solid improvement and aligns with the AUTO/EPOLL/KQUEUE/IO_URING/NIO semantics described in the PR.
One nuance: the fallback decisions for channel class and IoHandler are made independently. In rare cases where, for example,
ioUringAvailable()returns true butcreateHandler("...IoUringIoHandler")fails (class missing ornewFactorythrows),initGroupswill fall back toNioIoHandler, whilestartAsynccould still choose anIoUringServerSocketChannel(ifloadChannelClasssucceeds). That would pair a native channel with a NIO handler, which is likely unsupported.To make fallbacks strictly consistent, consider resolving an “effective transport” once and basing both selections on that, or coupling them more tightly, e.g.:
- compute a resolved
TransportType effectiveTransportthat reflects availability and reflection success,- use that for both
channelClassandhandler, so any failure in either path cleanly downgrades to NIO for both.This keeps the guarantees around explicit vs AUTO modes and ensures there is no scenario where channel and IoHandler disagree on the underlying transport.
Also applies to: 218-238, 265-302, 304-315, 404-439
239-263: GuardFixedRecvByteBufAllocatoragainst non-positive buffer sizesApplying socket options in
applyConnectionOptionslooks good overall, and wiringRECVBUF_ALLOCATORfromgetTcpReceiveBufferSize()is a nice touch.Given Netty’s
FixedRecvByteBufAllocatorexpects a strictly positive buffer size, it may be safer to tighten the guard:- if (config.getTcpReceiveBufferSize() != -1) { - bootstrap.childOption(ChannelOption.SO_RCVBUF, config.getTcpReceiveBufferSize()); - bootstrap.childOption(ChannelOption.RECVBUF_ALLOCATOR, new FixedRecvByteBufAllocator(config.getTcpReceiveBufferSize())); - } + if (config.getTcpReceiveBufferSize() > 0) { + bootstrap.childOption(ChannelOption.SO_RCVBUF, config.getTcpReceiveBufferSize()); + bootstrap.childOption(ChannelOption.RECVBUF_ALLOCATOR, + new FixedRecvByteBufAllocator(config.getTcpReceiveBufferSize())); + }This avoids passing zero or negative values through to the allocator even if a caller misconfigures the socket config.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/BasicConfiguration.java(4 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java(5 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/ack/AckManager.java(3 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/handler/ClientHead.java(4 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/handler/ClientsBox.java(1 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java(4 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/namespace/NamespacesHub.java(1 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/protocol/JacksonJsonSupport.java(2 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/scheduler/HashedWheelScheduler.java(1 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/scheduler/HashedWheelTimeoutScheduler.java(2 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/HazelcastPubSubStore.java(2 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/MemoryStore.java(1 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/MemoryStoreFactory.java(2 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/RedissonPubSubStore.java(2 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/HazelcastStoreTest.java(1 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/RedissonStoreTest.java(1 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/namespace/NamespacesHub.java (1)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/misc/CompositeIterable.java (1)
CompositeIterable(22-56)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/handler/ClientsBox.java (1)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/HandshakeData.java (1)
HandshakeData(26-130)
🪛 ast-grep (0.40.0)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java
[info] 245-245: "Detected use of a Java socket that is not encrypted. As a result, the
traffic could be read by an attacker intercepting the network traffic. Use
an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory'
instead."
Context: new ServerSocket(0)
Note: [CWE-319] Cleartext Transmission of Sensitive Information [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(unencrypted-socket-java)
🔇 Additional comments (28)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/handler/ClientsBox.java (1)
20-29: ConcurrentHashMap migration is correct and keeps semantics intactSwitching from Netty’s
PlatformDependent.newConcurrentHashMap()to plainnew ConcurrentHashMap<>()foruuid2clientsandchannel2clientspreserves the existing concurrency behavior (thread‑safe put/get/remove) while simplifying dependencies. No API or functional changes for callers; this is a clean internal refactor.netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/RedissonPubSubStore.java (1)
19-22: ConcurrentHashMap replacement is appropriate and keeps behavior intactSwitching from
PlatformDependent.newConcurrentHashMap()tonew ConcurrentHashMap<>()while keeping the field typed asConcurrentMap<String, Queue<Integer>>preserves the concurrency semantics and removes the dependency on Netty’sPlatformDependent. No issues here from a correctness or performance standpoint in this context.Also applies to: 38-38
netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/MemoryStoreFactory.java (1)
20-20: LGTM! Clean migration to standard JDK.The replacement of
PlatformDependent.newConcurrentHashMap()with standardConcurrentHashMapsimplifies dependencies and aligns with the broader migration pattern across the codebase.Also applies to: 50-50
netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/MemoryStore.java (1)
19-23: Switch to JDKConcurrentHashMapis appropriate and completeUsing
java.util.concurrent.ConcurrentHashMapis a clean, standard replacement for Netty's internal helper. Verification confirms no remaining usages ofPlatformDependent.newConcurrentHashMapin netty-socketio-core. The API and concurrency characteristics remain appropriate for this use case.netty-socketio-core/src/main/java/com/corundumstudio/socketio/namespace/NamespacesHub.java (2)
21-21: LGTM: Standard JDK import added.The import is necessary for the explicit
ConcurrentHashMapinstantiation on line 31.
31-31: Good simplification to standard JDK.Replacing
PlatformDependent.newConcurrentHashMap()withnew ConcurrentHashMap<>()reduces dependency on Netty internals for standard concurrency utilities. ModernConcurrentHashMapis well-optimized and appropriate for this use case. Retaining theConcurrentMapinterface declaration is good design.netty-socketio-core/src/main/java/com/corundumstudio/socketio/scheduler/HashedWheelTimeoutScheduler.java (2)
27-27: LGTM: Standard library import added.The addition of
ConcurrentHashMapimport is appropriate and necessary for the simplified initialization approach.
39-39: LGTM: Simplified to standard ConcurrentHashMap.The migration from
PlatformDependent.newConcurrentHashMap()to the standardConcurrentHashMapsimplifies the code and reduces dependency on Netty internals. Modern Java'sConcurrentHashMapis well-optimized and suitable for this use case. The field type correctly remains theConcurrentMapinterface.netty-socketio-core/src/main/java/com/corundumstudio/socketio/scheduler/HashedWheelScheduler.java (1)
19-19: LGTM! Clean refactoring to standard JDK collections.The replacement of
PlatformDependent.newConcurrentHashMap()withnew ConcurrentHashMap<>()is correct and beneficial. This reduces coupling to Netty's internal utilities while maintaining identical thread-safe behavior. Modern ConcurrentHashMap is highly optimized and well-suited for this use case.Also applies to: 30-30
netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/HazelcastPubSubStore.java (2)
19-19: LGTM!Import addition is correct and necessary for the ConcurrentHashMap instantiation below.
38-38: LGTM!The migration from
PlatformDependent.newConcurrentHashMap()to standardConcurrentHashMapis functionally correct and maintains the same thread-safety guarantees. For this subscription registry use case, the standard implementation is sufficient.netty-socketio-core/src/main/java/com/corundumstudio/socketio/protocol/JacksonJsonSupport.java (1)
29-29: LGTM! Clean migration to standard JDK concurrent collections.The replacement of
PlatformDependent.newConcurrentHashMap()withnew ConcurrentHashMap<>()is correct and maintains the same thread-safety guarantees. This reduces dependency on Netty internal utilities while providing identical concurrent map semantics for theeventMappingfield.Also applies to: 169-169
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java (6)
3-9: LGTM - JavaDoc formatting improved.The addition of
<p>paragraph tags follows proper JavaDoc HTML formatting conventions and will improve documentation rendering.
34-34: LGTM - Necessary imports added.The imports are properly used:
@NotNullfor the new helper method (line 174) andChannelHandlerContextfor the existing exception handler (line 106).Also applies to: 50-51
62-62: LGTM - Immutability improved.Marking these fields as
finalis a good practice since they're initialized once and never reassigned.Also applies to: 64-64, 66-66, 70-70
162-172: LGTM - Good refactoring.Extracting HTTP connection setup into the helper method improves code organization and reusability.
211-211: LGTM - Code quality improvements.The changes improve code quality: assertion order now follows JUnit convention (expected first), and
String.joinis cleaner than the stream-based approach.Also applies to: 231-231, 233-233
238-238: LGTM - Resource management improved.The addition of the JavaDoc
<p>tag is proper formatting, and the try-with-resources pattern ensures theServerSocketis properly closed even if an exception occurs during port retrieval.Note: The static analysis warning about unencrypted socket is a false positive. This is test utility code for finding a free port on localhost—no sensitive data is transmitted, and SSL is not applicable for port discovery.
Also applies to: 246-249
netty-socketio-core/src/main/java/com/corundumstudio/socketio/namespace/Namespace.java (3)
27-27: LGTM: Necessary imports for refactoring.The added imports support the replacement of Netty's PlatformDependent utilities with standard JDK concurrent collections and introduce SLF4J logging capabilities to the namespace.
Also applies to: 31-32
76-76: LGTM: Clean refactoring to standard JDK collections.Replacing
PlatformDependent.newConcurrentHashMap()withnew ConcurrentHashMap<>()is appropriate here. Modern JVM implementations provide robust concurrent collections, and this change reduces coupling to Netty's internal utilities while maintaining identical thread-safety guarantees and performance characteristics.Also applies to: 85-87
378-378: LGTM: Correct concurrent set idiom.Using
Collections.newSetFromMap(new ConcurrentHashMap<>())is the standard and correct approach for creating a thread-safe concurrent set in the JDK. This maintains the same concurrency guarantees as the previous implementation.netty-socketio-core/src/main/java/com/corundumstudio/socketio/handler/ClientHead.java (2)
27-65: ConcurrentHashMap fornamespaceClientsis appropriate hereSwitching
namespaceClientstoConcurrentHashMapmatches the multi-threaded access patterns (addNamespaceClient,removeNamespaceClient, iteration inonChannelDisconnect) and removes the dependency on Netty internals without changing semantics. Looks good.
120-155: NarrowingcatchfromThrowabletoExceptionis a good safety improvementCatching
Exceptioninstead ofThrowableinreleasePollingChannel,cancelPing, andcancelPingTimeoutavoids inadvertently swallowing serious JVM errors while still protecting the control flow from operational failures. The logging keeps diagnostics intact.netty-socketio-core/src/main/java/com/corundumstudio/socketio/ack/AckManager.java (1)
22-23: Using JDKConcurrentHashMapinstead of Netty PlatformDependent maps is fineBoth
ackCallbacksandackEntriesnow useConcurrentHashMap, which preserves the concurrent semantics and removes the dependency on Netty internals. The rest of the code already treats them asMap/ConcurrentMap, so this is a clean drop-in replacement.Also applies to: 45-46, 74-75
netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (4)
98-120: Broadcast/room operations changes are safeThe added null checks and the explicit
new ArrayList<>()usage ingetBroadcastOperationsandgetRoomOperationsare fine and keep behavior intact while avoiding potential NPEs ifnamespacesHub.getAllNamespaces()ever returns null.
317-330: Stop state transition and logging are consistentUsing
compareAndSet(STARTED, STOPPING)instop()plus the finalserverStatus.set(ServerStatus.INIT)gives you a clear lifecycle and prevents double-stop. The new warning log with the current state is helpful when debugging misuse.
399-402: Listener overload signature change is non-breakingThe
addListeners(Object listeners, Class<?> clazz)overload simply forwards to the namespace and only changes the parameter name, so it’s a source-compatible change with no behavioral impact.
1-15: Header and helper additions look fine
- The minor Javadoc formatting tweaks (
<p>) are fine.- New Netty imports (
IoHandlerFactory,MultiThreadIoEventLoopGroup,NioIoHandler, etc.) align with the new native-transport handling code.- The reflection helpers (
isClassPresent,isNativeAvailable) are straightforward and non-invasive; they safely return false on any failure.No issues here.
Also applies to: 39-49, 404-415
netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/HazelcastStoreTest.java
Show resolved
Hide resolved
netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/RedissonStoreTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/AbstractStoreTest.java (2)
42-71: Static sharedGenericContainerlifecycle is efficient but check subclass interactions and concurrencySharing a static
GenericContainer<?>and starting it lazily insetUp()significantly reduces container startup cost, but a few details are worth verifying:
- If multiple concrete test classes extend
AbstractStoreTestand each overridescreateContainer()with different images/configs, only the first one to run will actually create thecontainer; later subclasses will silently reuse that same instance. That can lead to subtle cross-test interference. If that scenario exists, consider either:
- Moving the static
containerto each concrete subclass, or- Keeping
containernon-static here and letting subclasses manage reuse if needed.- Lazy initialization of the static field is not synchronized. If you ever enable parallel test execution at the class or method level, two threads could race to create/start the container. This likely won’t manifest under default sequential execution, but it’s something to keep in mind.
- The static
@AfterAll cleanup()on an abstract base class looks fine conceptually, but please confirm in your JUnit 5 setup that it is indeed invoked once per concrete subclass so the container is always closed as intended.If all current subclasses share the same container configuration and tests run sequentially, this design is acceptable; otherwise, it may be worth tightening the lifecycle boundaries as above.
222-235:equalschange is correct;hashCodecan optionally be modernizedUsing
Objects.equals(name, that.name)keeps theequalsimplementation null-safe and aligned with the previous behavior, and it stays consistent with the currenthashCode(which already incorporates bothnameandvalue).As a small optional cleanup, you could modernize
hashCodeto useObjects.hashfor symmetry and readability:@Override public int hashCode() { return Objects.hash(name, value); }Not required for correctness, but it keeps the pair
equals/hashCodestylistically consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/AbstractStoreTest.java(6 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/HazelcastStoreTest.java(2 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/RedissonStoreTest.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/HazelcastStoreTest.java
- netty-socketio-core/src/test/java/com/corundumstudio/socketio/store/RedissonStoreTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (25) / build
- GitHub Check: build (21) / build
- GitHub Check: build (17) / build
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/HazelcastPubSubStore.java (1)
75-82: Guard against nullregIdsinunsubscribe
map.remove(name)can returnnullifunsubscribeis invoked for a type with no registrations (or if called twice), which would cause aNullPointerExceptionin thefor (UUID id : regIds)loop. This is pre-existing but easy to harden:public void unsubscribe(PubSubType type) { String name = type.toString(); - Queue<UUID> regIds = map.remove(name); - ITopic<Object> topic = hazelcastSub.getTopic(name); - for (UUID id : regIds) { - topic.removeMessageListener(id); - } + Queue<UUID> regIds = map.remove(name); + if (regIds == null || regIds.isEmpty()) { + return; + } + ITopic<Object> topic = hazelcastSub.getTopic(name); + for (UUID id : regIds) { + topic.removeMessageListener(id); + } }This keeps behavior identical when there are listeners but avoids an unnecessary NPE on defensive/unpaired calls to
unsubscribe.
🧹 Nitpick comments (3)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java (2)
34-51: Clean up extra blank line.Line 51 contains an unnecessary extra blank line that should be removed for consistency.
Apply this diff:
import io.netty.channel.ChannelHandlerContext; - import static org.junit.jupiter.api.Assertions.assertEquals;
174-192: Consolidate duplicate null checks.The method checks
bodyForPost != nulltwice (lines 175 and 180), which is redundant and reduces readability.Apply this diff to simplify the logic:
private static @NotNull HttpURLConnection getHttpURLConnection(String bodyForPost, HttpURLConnection http) throws IOException { if (bodyForPost != null) { http.setRequestMethod("POST"); // PUT is another valid option http.setDoOutput(true); - } - - if (bodyForPost != null) { byte[] out = bodyForPost.getBytes(StandardCharsets.UTF_8); http.setFixedLengthStreamingMode(out.length); http.setRequestProperty("Content-Type", "application/x-www-form-urlencoded; charset=UTF-8"); http.connect(); try (OutputStream os = http.getOutputStream()) { os.write(out); } } else { http.connect(); } return http; }netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (1)
144-185: LGTM! NIO case properly handled; consider adding INFO logging for AUTO transport selection.The explicit
case NIO:at lines 170-172 correctly addresses the past critical issue—users selecting NIO will now get pure NIO instead of falling through to AUTO.The AUTO case (lines 174-185) gracefully tries native transports in priority order and falls back to NIO. However, it doesn't log which transport was ultimately selected. Adding an INFO-level log after each successful branch would improve observability during server startup.
Optional improvement—log the selected transport:
case AUTO: default: if (ioUringAvailable()) { channelClass = loadChannelClass("io.netty.channel.uring.IoUringServerSocketChannel"); + log.info("AUTO selected IO_URING transport"); } else if (epollAvailable()) { channelClass = loadChannelClass("io.netty.channel.epoll.EpollServerSocketChannel"); + log.info("AUTO selected EPOLL transport"); } else if (kqueueAvailable()) { channelClass = loadChannelClass("io.netty.channel.kqueue.KQueueServerSocketChannel"); + log.info("AUTO selected KQUEUE transport"); } else { channelClass = NioServerSocketChannel.class; + log.info("AUTO selected NIO transport"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
header.txt(1 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java(4 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/nativeio/TransportType.java(1 hunks)netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/HazelcastPubSubStore.java(2 hunks)netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- header.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- netty-socketio-core/src/main/java/com/corundumstudio/socketio/nativeio/TransportType.java
🧰 Additional context used
🪛 ast-grep (0.40.0)
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java
[info] 245-245: "Detected use of a Java socket that is not encrypted. As a result, the
traffic could be read by an attacker intercepting the network traffic. Use
an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory'
instead."
Context: new ServerSocket(0)
Note: [CWE-319] Cleartext Transmission of Sensitive Information [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(unencrypted-socket-java)
🔇 Additional comments (11)
netty-socketio-core/src/main/java/com/corundumstudio/socketio/store/HazelcastPubSubStore.java (1)
19-21: ConcurrentHashMap migration looks correctUsing
ConcurrentMap<String, Queue<UUID>>backed by a plainnew ConcurrentHashMap<>()is a clean, type-safe replacement for the previous PlatformDependent-based map and keeps the concurrency semantics appropriate for this usage. The newUUIDimport also matches the existing listener registration IDs.As a follow-up, please just confirm other
PlatformDependent.newConcurrentHashMapusages across the module have been similarly updated for consistency.Also applies to: 38-38
netty-socketio-core/src/test/java/com/corundumstudio/socketio/transport/HttpTransportTest.java (5)
62-70: LGTM! Good practice to make fields final.Making these test fixtures final improves immutability and clearly communicates intent.
139-140: LGTM! URI construction is correct.The multi-line formatting improves readability for the long parameter list.
211-211: LGTM! Correct assertion parameter order.Following the JUnit convention of
assertEquals(expected, actual)improves test readability and error messages.
231-231: LGTM! Cleaner string concatenation.Using
String.joinis more idiomatic and readable than stream-based concatenation for this use case.
238-249: LGTM! Try-with-resources is best practice.The conversion to try-with-resources ensures proper socket cleanup and the added
<p>tag improves Javadoc formatting.Regarding the static analysis warning about unencrypted socket: this is a false positive. The
ServerSocketis only used to find a free port on localhost for testing purposes and is immediately closed without transmitting any data.netty-socketio-core/src/main/java/com/corundumstudio/socketio/SocketIOServer.java (5)
218-237: LGTM! Safe reflection-based transport loading with proper fallback.The reflection-based approach in
loadChannelClass(lines 218-227) safely handles optional native transport dependencies. Catching all exceptions and falling back to NIO is appropriate here—even if a native transport class is detected as available, classloading or initialization issues will gracefully degrade to NIO with a warning log.The helper methods
fallbackandfallbackFactory(lines 229-237) provide consistent logging and fallback behavior across both channel class and IoHandler selection paths.
249-249: LGTM! Correct Netty constant name.Using
ChannelOption.RECVBUF_ALLOCATORis correct—this aligns with Netty's naming convention for the receive buffer allocator option.
265-302: LGTM! IoHandler selection mirrors channel class selection logic.The switch statement for IoHandler selection (lines 269-298) correctly mirrors the channel class selection logic in
startAsync(), ensuring channel type and IoHandler are always paired consistently.The AUTO case (lines 287-297) includes a fallback warning log when reverting to NIO IoHandler, which complements the earlier suggestion to log successful AUTO selections in the channel switch for symmetry.
304-315: LGTM! Robust IoHandler factory creation with proper error handling.The
createHandlermethod (lines 304-315) uses reflection to dynamically invokenewFactory()on native IoHandler classes. The three-step reflection process (forName → getMethod → invoke) catches more failure modes than just class loading, and the early availability check (line 305) avoids unnecessary reflection attempts. Fallback to NIO IoHandler is always safe.
408-439: LGTM! Well-structured native transport detection using safe reflection.The native transport detection methods (lines 408-439) use a clean two-step pattern:
isClassPresentchecks if the native transport class can be loaded (lines 408-415)isNativeAvailableinvokes the transport's runtimeisAvailable()check (lines 417-424)Each transport-specific method (ioUringAvailable, epollAvailable, kqueueAvailable) combines both checks to ensure the native library is both present and functional. The safe reflection approach with catch-all exception handling (returning false) ensures optional dependencies never cause startup failures.
new files Copyrighted to The Socketio4j Project, to credit parent repo its copyrights are also added
|
@NeatGuyCoding Please check these files and review, most of them license changes |
|
Reviewing, thanks @sanjomo |
|
LGTM |
|
@NeatGuyCoding you can approve and merge |
Description
This PR introduces a unified and modern way to configure native I/O transports in netty-socketio by replacing the old boolean flags with a single
TransportTypeenum.The transport layer can now be:
NIO,EPOLL,KQUEUE,IOURING), orAUTO) based on the best available native transport at runtime.This simplifies configuration, improves platform correctness, and ensures consistent fallback behavior.
Type of Change
Related Issue
NA
Changes Made
Introduced new
TransportTypeenum:AUTO,NIO,EPOLL,KQUEUE,IOURINGRemoved legacy flags:
useLinuxNativeEpoll()useLinuxNativeIoUring()useUnixNativeKqueue()Added unified configuration option:
transportType = TransportType.AUTO(default)Implemented automatic native transport detection:
Updated event loop initialization to use enum-based selection
Updated server channel selection logic to use enum-based selection
Improved fallback behavior with consistent warning logs
Updated documentation and JavaDoc to reflect new configuration model
Refactored duplicate fallback logic using helper methods (where applicable)
Testing
Checklist
Additional Notes
NIOis selected.Summary by CodeRabbit
New Features
Refactor
Tests
Chores