diff mbox

validation: timer: fix handle unused tmo

Message ID 1463081754-26555-1-git-send-email-maxim.uvarov@linaro.org
State Accepted
Commit 5f49cc35fdee36417a239f6c8dcc1181f8e6f3e9
Headers show

Commit Message

Maxim Uvarov May 12, 2016, 7:35 p.m. UTC
Current validation test can produce bug:
timer.c:250:handle_tmo():Wrong tick: expected 18446744073709551615 actual 149
FAILED
    1. timer.c:246  - CU_FAIL("Wrong status (stale) for fresh timeout")
    2. timer.c:251  - CU_FAIL("odp_timeout_tick() wrong tick")
Which caused with wrong destroy sequence:
timer_free() calls timer_cancel(tp, idx, TMO_UNUSED).
which is: ((uint64_t)0xFFFFFFFFFFFFFFFF) or 18446744073709551615
and test calls handle_tmo() for event with TMO_UNUSED.
To fix reoder destroy sequence odp_timer_cancel()->sleep()->handle_tmo()->
->odp_timer_free().

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 test/validation/timer/timer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Bill Fischofer May 12, 2016, 8:14 p.m. UTC | #1
On Thu, May 12, 2016 at 2:35 PM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Current validation test can produce bug:

> timer.c:250:handle_tmo():Wrong tick: expected 18446744073709551615 actual

> 149

> FAILED

>     1. timer.c:246  - CU_FAIL("Wrong status (stale) for fresh timeout")

>     2. timer.c:251  - CU_FAIL("odp_timeout_tick() wrong tick")

> Which caused with wrong destroy sequence:

> timer_free() calls timer_cancel(tp, idx, TMO_UNUSED).

> which is: ((uint64_t)0xFFFFFFFFFFFFFFFF) or 18446744073709551615

> and test calls handle_tmo() for event with TMO_UNUSED.

> To fix reoder destroy sequence odp_timer_cancel()->sleep()->handle_tmo()->

> ->odp_timer_free().

>

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

>


Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org>



> ---

>  test/validation/timer/timer.c | 8 ++++++--

>  1 file changed, 6 insertions(+), 2 deletions(-)

>

> diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c

> index aad11aa..a396405 100644

> --- a/test/validation/timer/timer.c

> +++ b/test/validation/timer/timer.c

> @@ -397,8 +397,6 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)

>                         /* Cancel too late, timer already expired and

>                          * timeout enqueued */

>                         nstale++;

> -               if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)

> -                       CU_FAIL("odp_timer_free");

>         }

>

>         LOG_DBG("Thread %u: %" PRIu32 " timers set\n", thr, nset);

> @@ -429,6 +427,12 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)

>                         break;

>                 }

>         }

> +

> +       for (i = 0; i < allocated; i++) {

> +               if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)

> +                       CU_FAIL("odp_timer_free");

> +       }

> +

>         /* Check if there any more (unexpected) events */

>         odp_event_t ev = odp_queue_deq(queue);

>         if (ev != ODP_EVENT_INVALID)

> --

> 2.7.1.250.gff4ea60

>

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
Maxim Uvarov May 13, 2016, 1:52 p.m. UTC | #2
merged,
Maxim.

On 05/12/16 23:14, Bill Fischofer wrote:
>
>
> On Thu, May 12, 2016 at 2:35 PM, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     Current validation test can produce bug:
>     timer.c:250:handle_tmo():Wrong tick: expected 18446744073709551615
>     actual 149
>     FAILED
>         1. timer.c:246  - CU_FAIL("Wrong status (stale) for fresh
>     timeout")
>         2. timer.c:251  - CU_FAIL("odp_timeout_tick() wrong tick")
>     Which caused with wrong destroy sequence:
>     timer_free() calls timer_cancel(tp, idx, TMO_UNUSED).
>     which is: ((uint64_t)0xFFFFFFFFFFFFFFFF) or 18446744073709551615
>     and test calls handle_tmo() for event with TMO_UNUSED.
>     To fix reoder destroy sequence
>     odp_timer_cancel()->sleep()->handle_tmo()->
>     ->odp_timer_free().
>
>     Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>     <mailto:maxim.uvarov@linaro.org>>
>
>
> Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org 
> <mailto:bill.fischofer@linaro.org>>
>
>     ---
>      test/validation/timer/timer.c | 8 ++++++--
>      1 file changed, 6 insertions(+), 2 deletions(-)
>
>     diff --git a/test/validation/timer/timer.c
>     b/test/validation/timer/timer.c
>     index aad11aa..a396405 100644
>     --- a/test/validation/timer/timer.c
>     +++ b/test/validation/timer/timer.c
>     @@ -397,8 +397,6 @@ static void *worker_entrypoint(void *arg
>     TEST_UNUSED)
>                             /* Cancel too late, timer already expired and
>                              * timeout enqueued */
>                             nstale++;
>     -               if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)
>     -                       CU_FAIL("odp_timer_free");
>             }
>
>             LOG_DBG("Thread %u: %" PRIu32 " timers set\n", thr, nset);
>     @@ -429,6 +427,12 @@ static void *worker_entrypoint(void *arg
>     TEST_UNUSED)
>                             break;
>                     }
>             }
>     +
>     +       for (i = 0; i < allocated; i++) {
>     +               if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)
>     +                       CU_FAIL("odp_timer_free");
>     +       }
>     +
>             /* Check if there any more (unexpected) events */
>             odp_event_t ev = odp_queue_deq(queue);
>             if (ev != ODP_EVENT_INVALID)
>     --
>     2.7.1.250.gff4ea60
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
diff mbox

Patch

diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c
index aad11aa..a396405 100644
--- a/test/validation/timer/timer.c
+++ b/test/validation/timer/timer.c
@@ -397,8 +397,6 @@  static void *worker_entrypoint(void *arg TEST_UNUSED)
 			/* Cancel too late, timer already expired and
 			 * timeout enqueued */
 			nstale++;
-		if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)
-			CU_FAIL("odp_timer_free");
 	}
 
 	LOG_DBG("Thread %u: %" PRIu32 " timers set\n", thr, nset);
@@ -429,6 +427,12 @@  static void *worker_entrypoint(void *arg TEST_UNUSED)
 			break;
 		}
 	}
+
+	for (i = 0; i < allocated; i++) {
+		if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)
+			CU_FAIL("odp_timer_free");
+	}
+
 	/* Check if there any more (unexpected) events */
 	odp_event_t ev = odp_queue_deq(queue);
 	if (ev != ODP_EVENT_INVALID)