On 04/22/15 19:06, Ciprian Barbu wrote:
On Wed, Apr 22, 2015 at 5:13 PM, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
On 04/22/15 15:53, Bill Fischofer wrote:
It does that, but as Taras points out there is a race condition. If one
thread does an odp_pool_destroy() and it succeeds, another thread could do
an odp_pool_create() before the second odp_pool_destroy() and get the same
pool handle which would then be deleted by the second odp_pool_destroy()
call.
odp_pool_destroy() should hold lock inside it. (i.e. it already does that).
The scenario is not about a race condition on create/delete, it's
actually even simpler:
th1: lock -> create_pool -> unlock
th1: lock -> destroy_pool -> unlock
th2: lock -> create_pool -> unlock
th1: lock -> destroy_pool -> unlock
Ok, but it's not undefined behavior. If you work with threads you should
know what you do.
So behavior is: destroy function fails if pool already destroyed.
I still think that Mikes test is valid. It's single threaded application
with very exact case.
Analogy for example:
char *a = malloc(10);
a[5] = 1;
On your logic it has to be undefined behavior due to some other thread
can free or malloc with different size.
Maxim.
On Wed, Apr 22, 2015 at 7:13 AM, Maxim Uvarov <maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>> wrote:
I think in that case double free of the pool *has to be defined*.
The other deal is that odp_pool_destroy(odp_pool_t pool_hdl)
should lock pool table, then
check if handle is valid
and if it's not valid - then return error.
Maxim.
On 04/22/15 14:26, Mike Holmes wrote:
On 22 April 2015 at 06:44, Taras Kondratiuk
<taras.kondrat...@linaro.org
<mailto:taras.kondrat...@linaro.org>
<mailto:taras.kondrat...@linaro.org
<mailto:taras.kondrat...@linaro.org>>> wrote:
On 04/21/2015 10:14 PM, Mike Holmes wrote:
Signed-off-by: Mike Holmes <mike.hol...@linaro.org
<mailto:mike.hol...@linaro.org>
<mailto:mike.hol...@linaro.org
<mailto:mike.hol...@linaro.org>>>
---
test/validation/odp_pool.c | 25
+++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/test/validation/odp_pool.c
b/test/validation/odp_pool.c
index 1a518a0..c2f9a1b 100644
--- a/test/validation/odp_pool.c
+++ b/test/validation/odp_pool.c
@@ -11,6 +11,30 @@ static int pool_name_number = 1;
static const int default_buffer_size = 1500;
static const int default_buffer_num = 1000;
+static void pool_double_destroy(void)
+{
+ odp_pool_param_t params = {
+ .buf = {
+ .size =
default_buffer_size,
+ .align =
ODP_CACHE_LINE_SIZE,
+ .num =
default_buffer_num,
+ },
+ .type = ODP_POOL_BUFFER,
+ };
+ odp_pool_t pool;
+ char pool_name[ODP_POOL_NAME_LEN];
+
+ snprintf(pool_name, sizeof(pool_name),
+ "test_pool-%d", pool_name_number++);
+
+ pool = odp_pool_create(pool_name, ODP_SHM_INVALID,
¶ms);
+ CU_ASSERT_FATAL(pool != ODP_POOL_INVALID);
+ CU_ASSERT(odp_pool_to_u64(pool) !=
+ odp_pool_to_u64(ODP_POOL_INVALID));
+ CU_ASSERT(odp_pool_destroy(pool) == 0);
+ CU_ASSERT(odp_pool_destroy(pool) < 0);
Is this an expected behavior?
Yes I think it could be if you delete a bad pool.
This test arose more specifically to stress the error leg of
this function using only the public API - our validation does
very little to quantify what is permissible beyond very
optimistic cases, and in part to clarify the behavior in the
docs when you do try to do those other things.
Do we have it documented somewhere?
Not yet but I want it to be.
If the test goes in then we can add the specific case that
deleted a destroyed pool returns an error, if not we can
document that it is undefined.
I assume behavior should be undefined in this case. After
pool is
destroyed its handle can't be used anymore.
We need to document that in words that that is the case.
This test is single-threaded, but assume that there is
another thread
which created a pool with the same odp_pool_t handle just
between
two odp_pool_destroy(pool) calls. A second
odp_pool_destroy() will
destroy a new pool and return 0.
If you demand this behavior, then you effectively force
implementation
to use generation-tagged handles.
-- Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/>***│ *Open source software
for ARM SoCs
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
