[YDBTest#598] Fix errors in SOCKET OPEN command to show up (GT.M V7.0-003 regression)
Background
Below is pasted from YDBTest!1927 (comment 1900857290)
-
@ern0 noted this confusing behavior while writing test cases for !1927.
-
This is a short test case, opening a listen port without and with some OPTIONS:
echo 0 $gtm_dist/mumps -run %XCMD 'open "socket":(listen="9999:TCP")::"SOCKET" use "socket"' echo 1 $gtm_dist/mumps -run %XCMD 'open "socket":(listen="9999:TCP":options="KEEPIDLE=0")::"SOCKET" use "socket"' echo 2 $gtm_dist/mumps -run %XCMD 'open "socket":(listen="9999:TCP":options="KEEPINTVL=0")::"SOCKET" use "socket"' echo 3
-
The result is:
0 1 2 %YDB-E-IONOTOPEN, Attempt to USE an I/O device which has not been opened 3
-
As you see, setting KEEPINTVL results that the socket is not being opened.
-
I could change
KEEPIDLE=0
toKEEPALIVE=0
or other parameters, produces no error, only withKEEPINTVL=0
. -
Tried to detect the error:
$ $gtm_dist/mumps -run %XCMD 'open "socket":(listen="9999:TCP":options="KEEPINTVL=0")::"SOCKET" set t=$test use $principal write "$test=",t,!' $test=1
...but it prints:
$test=1
. -
Is it a normal behaviour?
Issue
-
What @ern0 identified is indeed an issue in my understanding.
-
GT.M V7.0-003 (commit 35326517) had changed logic in
sr_port/iosocket_tcp_keepalive.c
that issued anrts_error()
iftrap
was TRUE to do it only iffreesocket && trap
was TRUE. -
freesocket
is a new parameter that is now passed intoiosocket_tcp_keepalive()
. And it is set toTRUE
in the calls fromiosocket_bind()
(which in turn is called fromiosocket_open()
). -
In case the caller is calling with
freesocket
set to TRUE, it assumes any errors internally iniosocket_tcp_keepalive()
would have issued anrts_error_csa()
call.In fact there are comments to this effect in 2 out of 3 callers of
iosocket_tcp_keepalive()
.- sr_port/iosocket_connect.c
- sr_port/iosocket_bind.c
Line 722 is an example comment.
sr_port/iosocket_connect.c
721: if (keepalive_opt && !iosocket_tcp_keepalive(sockptr, keepalive_opt, action, TRUE)) 722: return FALSE; /* iosocket_tcp_keepalive issues rts_error rather than return */
-
But in the example where
KEEPINTVL=0
is set in theOPEN
command, the rts_error is not being issued. -
This is because although
freesocket
is TRUE,trap
is FALSE (becauseIOERROR=TRAP
was not specified in the OPEN command). -
And so
freesocket && trap
evaluates to FALSE and therts_error()
call does not happen. -
This should have been
freesocket || trap
in my understanding. This is because iffreesocket
is TRUE, the caller assumes the rts_error happens in the callee and so irrespective of the value oftrap
, we should issue the error. -
To justify this reasoning, I looked for other usages of
freesocket
andtrap
in if conditions in the same line and found the following.$ rg 'freesocket.*trap' sr_port/iosocket_tcp_keepalive.c 161: if (freesocket && trap) sr_port/iosocket_sockopt.c 46: if (freesocket || trap) 77: if (freesocket || trap)
-
Notice the usages in lines 46 and 77 have
||
whereas the usage in line 161 has&&
.
Fix
-
The fix is simple and is to replace the
&&
in line 161 with a||
. -
With this change, the same test case that issued a confusing
IONOTOPEN
error above now issues a more accurateSETSOCKOPTERR
error.$ $gtm_dist/mumps -run %XCMD 'open "socket":(listen="9999:TCP":options="KEEPINTVL=0")::"SOCKET" use "socket"' %YDB-E-SETSOCKOPTERR, Setting the socket attribute TCP_KEEPINTVL failed: (errno == 22) Invalid argument