HPCC-36063 feat(jlib) Add support for Unix domain sockets by ghalliday · Pull Request #21146 · hpcc-systems/HPCC-Platform

ghalliday · 2026-03-25T16:26:18Z

Type of change:

This change is a bug fix (non-breaking change which fixes an issue).
This change is a new feature (non-breaking change which adds functionality).
This change improves the code (refactor or other change that does not change the functionality)
This change fixes warnings (the fix does not alter the functionality or the generated code)
This change is a breaking change (fix or feature that will cause existing behavior to change).
This change alters the query API (existing queries will have to be recompiled)

Checklist:

Smoketest:

Send notifications about my Pull Request position in Smoketest queue.
Test my draft Pull Request.

Testing:

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ghalliday · 2026-03-25T16:27:21Z

This did not seem to have any noticeable impact on roxie performance. I will retest on a v2 at a later date. I think worth merging though.

Copilot

Pull request overview

Adds Unix Domain Socket (UDS) support as an optional transport path, wiring it through jlib socket creation/connect paths and Roxie TCP transport helpers.

Changes:

Extended ISocket and secure socket utilities to optionally use AF_UNIX sockets.
Updated Roxie TCP sender/listener plumbing to switch to UDS when protocol=uds.
Updated unit tests call sites to match the new async-connect API signature.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
testing/unittests/socketasynctests.cpp	Updates async-connect socket creation calls to pass a UDS flag.
testing/unittests/multishotaccepttests.cpp	Updates listener constructor call to include new UDS parameter.
system/security/securesocket/socketutils.hpp	Extends listener/sender APIs with UDS options.
system/security/securesocket/socketutils.cpp	Implements UDS-aware listen/connect and async connect setup.
system/jlib/jsocket.hpp	Extends `ISocket` API for UDS and async connect.
system/jlib/jsocket.cpp	Adds AF_UNIX socket-mode support and UDS create/connect helpers.
roxie/udplib/udpsha.cpp	Adds global `useUdsTransport` configuration flag.
roxie/udplib/udplib.hpp	Exposes `useUdsTransport` flag.
roxie/udplib/tcptrs.cpp	Passes UDS flag into `CTcpSender`.
roxie/udplib/tcptrr.cpp	Updates listener start to pass UDS flag.
roxie/ccd/ccdqueue.cpp	Updates listener/sender startup to pass UDS flag.
roxie/ccd/ccdmain.cpp	Parses `protocol=uds` and enables the UDS transport flag.

Copilot · 2026-03-25T16:34:52Z

+        getSockAddrEndpoint(peerSockAddr, peerSockAddrLen, peerEp);
+    }
+
    CSocket *ret = new CSocket(newsock,sm_tcp,true,&peerEp);


When accepting an AF_UNIX connection, the accepted socket is still constructed with sm_tcp. This will misclassify the socket (e.g., mode-dependent behavior like connectionless() and any future mode checks). Consider selecting sm_unix when peerSockAddr.sa.sa_family == AF_UNIX (or derive from the listening socket’s sockmode) when constructing the accepted CSocket.

Suggested change

CSocket *ret = new CSocket(newsock,sm_tcp,true,&peerEp);

auto mode = sm_tcp;

#ifndef _WIN32

if (peerSockAddr.sa.sa_family == AF_UNIX)

mode = sm_unix;

#endif

CSocket *ret = new CSocket(newsock,mode,true,&peerEp);

That looks like a potential issue - I will revisit later.

Copilot · 2026-03-25T16:34:52Z

+inline socklen_t setUnixSockAddr(J_SOCKADDR &u, const IpAddress &ip, unsigned short port) {
+    StringBuffer ipStr;
+    ip.getIpText(ipStr);
+    ipStr.replace((char) '.', (char) '_');
+    ipStr.replace((char) ':', (char) '_');
+    u.sun.sun_family = AF_UNIX;
+    snprintf(u.sun.sun_path, sizeof(u.sun.sun_path), "/tmp/hpcc_uds_%s_%u.sock", ipStr.str(), port);
+    return sizeof(u.sun);
+}


The UDS path is predictable and created under /tmp, which is world-writable. This can enable denial-of-service and/or symlink-style attacks around bind/unlink, and also risks path collisions. Consider using a dedicated runtime directory with restrictive permissions (e.g., under /var/run or a configured directory), and/or validate ownership/permissions before unlinking, and set explicit socket file permissions after bind.

I think this is a valid point./ @mckellyln any suggestions /var/lib/HPCC-Systems/uds?

Interesting if it was a shared filesystem that could be seen across containers on the same host.
But otherwise yes, /var/run/HPCCSystems/uds seems like a good dir.

I sort of agree with a comment below - we can have different IPs but same host depending on the default interface and perhaps its better to use a hostname or something more of a chance of being the same.

Copilot · 2026-03-25T16:34:53Z

+        IpAddress bindIp;
+        if (targetip.isNull()) GetHostIp(bindIp); else bindIp = targetip;
+        ul = setUnixSockAddr(u, bindIp, hostport);


UDS bind path derivation depends on bindIp (host IP if targetip is null). On the client side, the path derivation uses the endpoint IP passed for connect. If the endpoint IP differs from GetHostIp() (e.g., 127.0.0.1 vs interface IP, hostname vs IP, etc.), client and server will compute different filesystem paths and connects will fail. Consider deriving the UDS path from a stable identifier independent of endpoint IP (e.g., port only, or an explicit configured socket path), or ensuring both sides use the same canonical IP when useUDS is enabled.

Suggested change

IpAddress bindIp;

if (targetip.isNull()) GetHostIp(bindIp); else bindIp = targetip;

ul = setUnixSockAddr(u, bindIp, hostport);

// Derive UDS path from a stable identifier (port) rather than host IP,

// to avoid mismatches between server and client UDS paths.

ul = setUnixSockAddr(u, hostport);

I agree with this point, not sure how common it is, but perhaps the hostname ?

Copilot · 2026-03-25T16:34:53Z

    // _useIOUring: true to enable io_uring multishot accept (if available), false to use traditional thread-based accept
    //              Defaults to true, but can be overridden by expert/@useIOUring configuration setting
-    CSocketConnectionListener(unsigned port, bool _useTLS, unsigned _inactiveCloseTimeoutMs, unsigned _maxListenHandlerSockets, bool _useIOUring = true);
+    CSocketConnectionListener(unsigned port, bool _useTLS, unsigned _inactiveCloseTimeoutMs, unsigned _maxListenHandlerSockets, bool _useIOUring, bool _useUDS);


The comment states _useIOUring defaults to true, but the constructor signature no longer provides a default and now requires both _useIOUring and _useUDS. Update the comment to match the API, or reintroduce default arguments (e.g., _useIOUring=true, _useUDS=false) if that’s the intended behavior.

Suggested change

CSocketConnectionListener(unsigned port, bool _useTLS, unsigned _inactiveCloseTimeoutMs, unsigned _maxListenHandlerSockets, bool _useIOUring, bool _useUDS);

// _useUDS: true to use Unix domain sockets, false to use TCP/IP (defaults to false)

CSocketConnectionListener(unsigned port, bool _useTLS, unsigned _inactiveCloseTimeoutMs, unsigned _maxListenHandlerSockets, bool _useIOUring=true, bool _useUDS=false);

Copilot · 2026-03-25T16:34:54Z

        return config.getClear();
    }

+    static constexpr bool useUDS = false;


UDS support is introduced in the async-connect path, but the unit tests hardcode useUDS = false, so the new UDS branch is not exercised. Consider adding at least one test variant that sets useUDS = true and validates async connect (and failure/timeout behaviors) against a UDS listener to cover the new transport mode.

Suggested change

static constexpr bool useUDS = false;

static constexpr bool useUDS = true;

Definitely for future work - currently this doesn't show a significant boost, so unlikely to be used in production.

Copilot · 2026-03-25T16:34:54Z

            size32_t addrlen = 0;

-            Owned<ISocket> socket = ISocket::createForAsyncConnect(ep, addrHolder.getRef(), addrlen);
+            Owned<ISocket> socket = ISocket::createForAsyncConnect(ep, addrHolder.getRef(), addrlen, useUDS);


UDS support is introduced in the async-connect path, but the unit tests hardcode useUDS = false, so the new UDS branch is not exercised. Consider adding at least one test variant that sets useUDS = true and validates async connect (and failure/timeout behaviors) against a UDS listener to cover the new transport mode.

github-actions · 2026-03-25T16:44:57Z

Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-36063

Jirabot Action Result:
Assigning user: gavin.halliday@lexisnexisrisk.com
Workflow Transition To: Merge Pending
Updated PR

jakesmith

@ghalliday - looks ok afaics, 1 minor comment and some bracing formatting/inconsistencies.

jakesmith · 2026-03-27T11:08:41Z

 }


+inline socklen_t setUnixSockAddr(J_SOCKADDR &u, const IpAddress &ip, unsigned short port) {


formatting: new non allman style bracing

jakesmith · 2026-03-27T11:09:17Z

-    sockaddrlen = setSockAddr(sockaddr, targetip, hostport);
-    T_SOCKET sock = ::socket(sockaddr.sa.sa_family, SOCK_STREAM, targetip.isIp4() ? 0 : PF_INET6);
+#ifndef _WIN32
+    if (sockmode == sm_unix_server || sockmode == sm_unix) {


formatting: mixed bracing style (non allman) , and various other places.

jakesmith · 2026-03-27T11:13:27Z


        bool isBatchRoxie = strisame(roxieMode, "batch");
-        useTcpTransport = strisame(protocol, "tcp");
+        useTcpTransport = strisame(protocol, "tcp") || strisame(protocol, "uds");


intentional I assume, but could do with comment to clarify

mckellyln · 2026-04-06T21:04:43Z

    {
-        socket.setown(ISocket::connect_timeout(ep, 5000));
+        if (sender.useUDS)
+            socket.setown(ISocket::unix_connect(ep));


why not use the time value as well ?

mckellyln

Looks good but do have a few comments inline,

HPCC-36063 feat(jlib) Add support for Unix domain sockets

2cad28c

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ghalliday requested review from Copilot, jakesmith and mckellyln March 25, 2026 16:26

ghalliday added the Feature label Mar 25, 2026

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Copilot started reviewing on behalf of ghalliday March 25, 2026 16:56 View session

jakesmith approved these changes Mar 27, 2026

View reviewed changes

mckellyln reviewed Apr 6, 2026

View reviewed changes

-    CSocket *ret = new CSocket(newsock,sm_tcp,true,&peerEp);
+    auto mode = sm_tcp;
+#ifndef _WIN32
+    if (peerSockAddr.sa.sa_family == AF_UNIX)
+        mode = sm_unix;
+#endif
+    CSocket *ret = new CSocket(newsock,mode,true,&peerEp);

	CSocketConnectionListener(unsigned port, bool _useTLS, unsigned _inactiveCloseTimeoutMs, unsigned _maxListenHandlerSockets, bool _useIOUring, bool _useUDS);
	// _useUDS: true to use Unix domain sockets, false to use TCP/IP (defaults to false)
	CSocketConnectionListener(unsigned port, bool _useTLS, unsigned _inactiveCloseTimeoutMs, unsigned _maxListenHandlerSockets, bool _useIOUring=true, bool _useUDS=false);

	static constexpr bool useUDS = false;
	static constexpr bool useUDS = true;

		}


		inline socklen_t setUnixSockAddr(J_SOCKADDR &u, const IpAddress &ip, unsigned short port) {

Conversation

ghalliday commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change:

Checklist:

Smoketest:

Testing:

Uh oh!

ghalliday commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mckellyln Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

jakesmith left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mckellyln left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghalliday commented Mar 25, 2026 •

edited

Loading

mckellyln Mar 30, 2026 •

edited

Loading