#49884 Improve nunc-stans test to detect socket errors sooner
Closed 3 years ago by spichugi. Opened 5 years ago by firstyear.
firstyear/389-ds-base nunc-stans-small-stress  into  master

@@ -128,11 +128,7 @@ 

      assert(connctx != NULL);

      if (NS_JOB_IS_TIMER(ns_job_get_output_type(job))) {

          do_logging(LOG_ERR, "conn_write: job [%p] timeout\n", job);

- #ifdef ATOMIC_64BIT_OPERATIONS

-         __atomic_add_fetch_8(&server_fail_count, 1, __ATOMIC_SEQ_CST);

- #else

-         PR_AtomicIncrement(&server_fail_count);

- #endif

+         __atomic_add_fetch_8(&server_fail_count, 1, __ATOMIC_ACQ_REL);

          conn_ctx_free(connctx);

          assert_int_equal(ns_job_done(job), 0);

          return;
@@ -177,11 +173,7 @@ 

      if (NS_JOB_IS_TIMER(ns_job_get_output_type(job))) {

          /* The event that triggered this call back is because we timed out waiting for IO */

          do_logging(LOG_ERR, "conn_read: job [%p] timed out\n", job);

- #ifdef ATOMIC_64BIT_OPERATIONS

-         __atomic_add_fetch_8(&server_fail_count, 1, __ATOMIC_SEQ_CST);

- #else

-         PR_AtomicIncrement(&server_fail_count);

- #endif

+         __atomic_add_fetch_8(&server_fail_count, 1, __ATOMIC_ACQ_REL);

          conn_ctx_free(connctx);

          assert_int_equal(ns_job_done(job), 0);

          return;
@@ -213,7 +205,7 @@ 

          } else {

              do_logging(LOG_ERR, "conn_read: read error for job [%p] %d: %s\n", job, PR_GetError(), PR_ErrorToString(PR_GetError(), PR_LANGUAGE_I_DEFAULT));

  #ifdef ATOMIC_64BIT_OPERATIONS

-             __atomic_add_fetch_8(&server_fail_count, 1, __ATOMIC_SEQ_CST);

+             __atomic_add_fetch_8(&server_fail_count, 1, __ATOMIC_ACQ_REL);

  #else

              PR_AtomicIncrement(&server_fail_count);

  #endif
@@ -226,11 +218,7 @@ 

          /* Didn't read anything */

          do_logging(LOG_DEBUG, "conn_read: job [%p] closed\n", job);

          /* Increment the success */

- #ifdef ATOMIC_64BIT_OPERATIONS

-         __atomic_add_fetch_8(&server_success_count, 1, __ATOMIC_SEQ_CST);

- #else

-         PR_AtomicIncrement(&server_success_count);

- #endif

+         __atomic_add_fetch_8(&server_success_count, 1, __ATOMIC_ACQ_REL);

          conn_ctx_free(connctx);

          assert_int_equal(ns_job_done(job), 0);

          return;
@@ -330,41 +318,25 @@ 

      if (len < 0) {

          /* PRErrorCode prerr = PR_GetError(); */

          do_logging(LOG_ERR, "FAIL: connection error, no data \n");

- #ifdef ATOMIC_64BIT_OPERATIONS

-         __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_SEQ_CST);

- #else

-         PR_AtomicIncrement(&client_fail_count);

- #endif

+         __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_ACQ_REL);

          goto done;

      } else if (len == 0) {

          do_logging(LOG_ERR, "FAIL: connection closed, no data \n");

- #ifdef ATOMIC_64BIT_OPERATIONS

-         __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_SEQ_CST);

- #else

-         PR_AtomicIncrement(&client_fail_count);

- #endif

+         __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_ACQ_REL);

          goto done;

      } else {

          /* Be paranoid, force last byte null */

          buffer[buflen - 1] = '\0';

          if (strncmp("this is a test!\n", buffer, strlen("this is a test!\n")) != 0) {

              do_logging(LOG_ERR, "FAIL: connection incorrect response, no data \n");

- #ifdef ATOMIC_64BIT_OPERATIONS

-             __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_SEQ_CST);

- #else

-             PR_AtomicIncrement(&client_fail_count);

- #endif

+             __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_ACQ_REL);

              goto done;

          }

      }

  

      struct timespec ts;

      clock_gettime(CLOCK_MONOTONIC, &ts);

- #ifdef ATOMIC_64BIT_OPERATIONS

-     __atomic_add_fetch_8(&client_success_count, 1, __ATOMIC_SEQ_CST);

- #else

-     PR_AtomicIncrement(&client_success_count);

- #endif

+     __atomic_add_fetch_8(&client_success_count, 1, __ATOMIC_ACQ_REL);

      do_logging(LOG_ERR, "PASS: %ld.%ld %d\n", ts.tv_sec, ts.tv_nsec, client_success_count);

  

  done:
@@ -385,11 +357,7 @@ 

          char *err = NULL;

          PR_GetErrorText(err);

          do_logging(LOG_ERR, "FAIL: Socket failed, %d -> %s\n", PR_GetError(), err);

- #ifdef ATOMIC_64BIT_OPERATIONS

-         __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_SEQ_CST);

- #else

-         PR_AtomicIncrement(&client_fail_count);

- #endif

+         __atomic_add_fetch_8(&client_fail_count, 1, __ATOMIC_ACQ_REL);

          goto done;

      }

  
@@ -401,20 +369,16 @@ 

          /* char *err = malloc(PR_GetErrorTextLength()); */

          char *err = NULL;

          PR_GetErrorText(err);

-         do_logging(LOG_ERR, "FAIL: cannot connect, timeout %d -> %s \n", PR_GetError(), err);

+         do_logging(LOG_ERR, "FAIL: cannot connect, timeout %d -> %s check nspr4/prerr.h \n", PR_GetError(), err);

          /* Atomic increment fail */

- #ifdef ATOMIC_64BIT_OPERATIONS

-         __atomic_add_fetch_8(&client_timeout_count, 1, __ATOMIC_SEQ_CST);

- #else

-         PR_AtomicIncrement(&client_timeout_count);

- #endif

+         __atomic_add_fetch_8(&client_timeout_count, 1, __ATOMIC_ACQ_REL);

          if (sock != NULL) {

              PR_Close(sock);

          }

          goto done;

      }

      /* Now write data. */

-     PR_Write(sock, data, strlen(data) + 1);

+     assert_true(PR_Write(sock, data, strlen(data) + 1) > 0);

      /* create the read job to respond to events on the socket. */

      assert_int_equal(ns_add_io_job(ns_job_get_tp(job), sock, NS_JOB_READ | NS_JOB_THREAD, client_response_cb, NULL, NULL), 0);

  
@@ -501,8 +465,8 @@ 

      PR_SetSocketOption(listenfd, &prsod);

      prsod.option = PR_SockOpt_Reuseaddr;

      PR_SetSocketOption(listenfd, &prsod);

-     PR_Bind(listenfd, &netaddr);

-     PR_Listen(listenfd, 32);

+     assert_true(PR_Bind(listenfd, &netaddr) == PR_SUCCESS);

+     assert_true(PR_Listen(listenfd, 32) == PR_SUCCESS);

  

      listen_job = server_listener_init(stp, listenfd);

  

While testing on a fresh machine (without ipv6) I noticed the ns test
would fail. This led me to improve the state of the ns stress test
code to remove the legacy atomic, and check assertions of the sockets.

Author: William Brown william@blackhats.net.au

The code looks good to me.
Please, create an issue for this and change the commit head and body accordingly...

And you have my ack with it :)

rebased onto e4831bc

5 years ago

Pull-Request has been merged by firstyear

5 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2943

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago
Metadata