[YDBTest#598] Fix various issues in SOCKET USE command (GT.M V7.0-003 regression)
Background
-
See !1524 (comment 1901072557) for more details. But basically the below test case failed an assert.
$ $gtm_dist/mumps -run %XCMD 'open "socket":(listen="9999:TCP":IOERROR="NOTRAP")::"SOCKET" use "socket":(options="KEEPINTVL=0") ' %YDB-F-ASSERT, Assert failed in sr_unix/gtm_fd_trace.c line 185 for expression (FALSE)
Issue
-
The issue was that a
close(socketptr->sd);
was done insr_port/iosocket_tcp_keepalive.c
after commit 0e138473 becausefreesocket
was TRUE. But in this case,iosocket_use()
had callediosocket_tcp_keepalive()
with a copy of the socket device (curr_socketptr
variable insr_port/iosocket_use.c
) while keeping the original socket device still accessible using the variablesocketptr
iniosocket_use.c
. This meant thatclose()
iniosocket_tcp_keepalive
closedcurr_socketptr->sd
and reset it to beFD_INVALID
but did not do the same to the copysocketptr->sd
. -
And when halting, the process looked at all open devices and found
socketptr->sd
as a valid fd and so tried to close it resulting in theclose()
failing and causing the assert. -
There is a pre-existing comment which says
SOCKET_FREE
should not be used on the copy (line 537 below).sr_port/iosocket_use.c
536 /* Make a copy of the socket_struct so most errors leave the previous structure as is. 537 * SOCKET_FREE should not be used on this copy since it frees other things such as buffers 538 * DELIMITER is an exception due to how iosocket_delimiter manages storage.
-
Inspite of this, we end up doing a
SOCKET_FREE()
on the copy when called throughiosocket_tcp_keepalive()
iniosocket_use()
. -
This is incorrect and is the cause of the issue.
-
Additionally, after the call to
iosocket_tcp_keepalive()
,iosocket_use.c
does afree(curr_socketptr)
again. This would end up being a double-free.
Fix
-
The fix is simple and is to pass
FALSE
as thefreesocket
argument toiosocket_tcp_keepalive()
insr_port/iosocket_use.c
. -
And to do the
close(socketptr->sd)
andSOCKET_FREE(socketptr)
calls only iffreesocket
argument is TRUE (previously this happened iffreesocket || trap
was TRUE). -
Note that we still issue a
rts_error
iffreesocket || trap
is TRUE. -
This will take care of all the above described problems.
Tests
-
Below were the tests done to test OPEN/USE with IOERROR=NOTRAP/TRAP. Including scenarios tested during commit 0e138473.
YDB>open "socket":(listen="9999:TCP":IOERROR="NOTRAP":options="KEEPINTVL=0")::"SOCKET" set x=$DEVICE zwrite x %YDB-E-SETSOCKOPTERR, Setting the socket attribute TCP_KEEPINTVL failed: (errno == 22) Invalid argument YDB>open "socket":(listen="9999:TCP":IOERROR="TRAP":options="KEEPINTVL=0")::"SOCKET" set x=$DEVICE zwrite x %YDB-E-SETSOCKOPTERR, Setting the socket attribute TCP_KEEPINTVL failed: (errno == 22) Invalid argument YDB>open "socket":(listen="9999:TCP":IOERROR="NOTRAP")::"SOCKET" use "socket":(options="KEEPINTVL=0") set x=$device use $principal zwrite x zshow "d" x="1,Invalid argument" /dev/pts/1 OPEN TERMINAL NOPAST NOESCA NOREADONLY TYPE WIDTH=124 LENG=51 TTSYNC NOHOSTSYNC socket OPEN SOCKET TOTAL=1 CURRENT=0 SOCKET[0]=h1715372292002 DESC=3 LISTENING PASSIVE NOTRAP PORT=9999 ZDELAY ZBFSIZE=1024 ZIBFSIZE=131072 NODELIMITER YDB>open "socket":(listen="9998:TCP":IOERROR="TRAP")::"SOCKET" use "socket":(options="KEEPINTVL=0") %YDB-E-SETSOCKOPTERR, Setting the socket attribute TCP_KEEPINTVL failed: (errno == 22) Invalid argument