[Vortex] [BUG] Race condition around ctx->profiles_list

Francis Brosnan Blazquez francis at aspl.es
Wed Sep 8 20:04:16 CEST 2010


Hi Benoit,

Thanks for the patch. Here are some comments:

- I've restored vortex_profiles_get_actual_list_ref because it is a
public API in use and because this function do not race if the user
install all profiles to be used at the initialization code and then do
all the work with them (which is a usual pattern).

- vortex_profiles_has_profiles implementation has a bug which always
causes to return axl_true. This is because axl_list_length > 0 must be
used instead of checking the pointer to be not null.

- I've removed all calls to assert. They are too hard. This should be
done by the application level code. 

- There is a bug inside vortex_profiles_get_actual_list_ref and
vortex_profiles_acquire which do not check ctx reference before
acquiring the mutex. This bug was corrected on 1.1 series in the case of
vortex_profiles_get_actual_list_ref.

- I've updated vortex_profiles_release to check the ctx reference to
avoid null pointer exception. I see your intention by placing an assert
to check list reference received, but this is problematic because a
different pointer to the same list may be received. I've removed it. In
the same direction, because  we have one list per context, I've removed
"list" parameter to avoid requiring it from user.

- The code required to update vortex_ctx_private.h to include the mutex
profiles_list_mutex and the code to initialize and destroy it is missing
in the patch. I've implemented it.

- I've applied several indentation changes to conform with current
coding style. Please, try to adapt patches to conform this.

Having fixed these issues, the reg test do not run. After some checks I
found a bug introduced at vortex_greetings_send which now check for
having profiles registered instead of just checking for having
initialized the profile list. 

I've reintroduced the call to vortex_profiles_get_actual_list_ref to
avoid using vortex_profiles_has_profiles at this point. The same applies
to vortex_greetings_client_send. The idea is that we can send empty
greetings to this should succeed without having profiles in place.

After these changes the path work as expected and passes all regression
tests. I've also ported the path into 1.1 branch. Both branches passes
reg tests.

Cheers!

> Hello,
> 
> On Thu, Aug 19, 2010 at 8:09 PM, Francis Brosnan Blazquez
> <francis at aspl.es> wrote:
> > Ok. Your proposal looks good to me. This will also ensure that a node
> > inside the list, representing a profile name, is not terminated by an
> > unregister operation...
> 
> Attached patch should fix the issue.
> It's against vortex 1.0 but the idea is godd against 1.1 too.
> 
> Thank you,
> Benoît
> _______________________________________________
> Vortex mailing list
> Vortex at lists.aspl.es
> http://lists.aspl.es/cgi-bin/mailman/listinfo/vortex
-- 
Francis Brosnan Blazquez <francis at aspl.es>
ASPL



More information about the Vortex mailing list