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 > > >> 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
