Skip to content

[YDBTest#598] Fix errors in SOCKET OPEN command to show up (GT.M V7.0-003 regression)

Narayanan Iyer requested to merge nars1/YDB:ydbtest598 into master

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 to KEEPALIVE=0 or other parameters, produces no error, only with KEEPINTVL=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 an rts_error() if trap was TRUE to do it only if freesocket && trap was TRUE.

  • freesocket is a new parameter that is now passed into iosocket_tcp_keepalive(). And it is set to TRUE in the calls from iosocket_bind() (which in turn is called from iosocket_open()).

  • In case the caller is calling with freesocket set to TRUE, it assumes any errors internally in iosocket_tcp_keepalive() would have issued an rts_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 the OPEN command, the rts_error is not being issued.

  • This is because although freesocket is TRUE, trap is FALSE (because IOERROR=TRAP was not specified in the OPEN command).

  • And so freesocket && trap evaluates to FALSE and the rts_error() call does not happen.

  • This should have been freesocket || trap in my understanding. This is because if freesocket is TRUE, the caller assumes the rts_error happens in the callee and so irrespective of the value of trap, we should issue the error.

  • To justify this reasoning, I looked for other usages of freesocket and trap 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 accurate SETSOCKOPTERR 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

Merge request reports

Loading