Opened 8 years ago

Closed 7 years ago

#2028 closed defect (fixed)

socket session forward test fails on solaris11

Reported by: jinmei Owned by: jinmei
Priority: high Milestone: Sprint-20120619
Component: ~Inter-module communication(obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 7 Add Hours to Ticket: 0
Total Hours: 4.5 Internal?: no

Description

See this http://git.bind10.isc.org/~tester/builder/BIND10/20120607091008-Solaris11-i386-GCC/logs/unittests.out

I also confirmed it's always reproducible. However, I'm afraid it's
unlikely to be fixable by an urgent care patch, and the debug and fix
will take some time. So I suggest we do it as a regular bug fix at
the highest priority in the next sprint.

Subtickets

Attachments (1)

regression-test.cc (2.8 KB) - added by jinmei 8 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by jreed

[ RUN      ] ForwardTest.pushAndPop
socketsession_unittest.cc:532: Failure
Value of: recvfrom(fwd_fd, recvbuf, sizeof(recvbuf), setRecvDelay(fwd_fd), convertSockAddr(&ss), &sa_len)
  Actual: -1
Expected: sizeof(recvbuf)
Which is: 12
Google Test trace:
socketsession_unittest.cc:577: Passing UDP/IPv6 session with large data
socketsession_unittest.cc:539: Failure
Value of: string(recvbuf)
  Actual: ""
Expected: string(TEST_DATA)
Which is: "BIND10 test"
Google Test trace:
socketsession_unittest.cc:577: Passing UDP/IPv6 session with large data
[  FAILED  ] ForwardTest.pushAndPop (9996 ms)

also see ticket #1981

comment:2 Changed 8 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120619
  • Priority changed from very high to high

comment:3 Changed 8 years ago by jinmei

  • Owner set to jinmei
  • Status changed from new to accepted

Changed 8 years ago by jinmei

comment:4 Changed 8 years ago by jinmei

trac2028 is ready for review.

The attached file (additions to socketsession_unittest.cc) highlights
the issue more clearly. To see the effect you'd also need to disable
SO_REUSEADDR:

#if 0
        const int on = 1;
        if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) == -1) {
            isc_throw(isc::Unexpected, "setsockopt(SO_REUSEADDR) failed: " <<
                      strerror(errno));
        }
#endif

I've not figured out how exactly that happened, but what happened was
that when we create a socket and forward the socket FD via a UNIX
domain socket (and then close both the original and forwarded FDs),
this deviant system somehow prevents the reuse of the port for some
short period (some shorter than 1sec).

So, my proposed fix is to just use different ports for the offending
tests, which is what this branch does.

Note that TCP tests aren't affected, probably because it forwards a
TCP socket on accept() whose remote address is always different.

Note also that this behavior of Solaris won't affect more practical
cases of BIND 10, because the forwarder side of app should keep the
UDP socket open, in which case this type of problem doesn't happen (I
confirmed that). So this is basically only for the specific set of
unit tests, and any workaround that avoids the trouble should be okay.

Since it's only about unit tests and for a newest buildbot
environment, I don't think we need a change log for it.

comment:5 Changed 8 years ago by jinmei

  • Owner changed from jinmei to UnAssigned
  • Status changed from accepted to reviewing

comment:6 Changed 7 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:7 follow-up: Changed 7 years ago by vorner

  • Owner changed from vorner to jinmei

Hello

While I couldn't check on solaris (as I don't have one), the fix and explanation seem to make sense. If you verified it, please merge.

Thank you

comment:8 in reply to: ↑ 7 Changed 7 years ago by jinmei

Replying to vorner:

Hello

While I couldn't check on solaris (as I don't have one), the fix and explanation seem to make sense. If you verified it, please merge.

Thanks for the review. Merge done, closing.

comment:9 Changed 7 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Total Hours changed from 0 to 4.5
Note: See TracTickets for help on using tickets.