nsd registration failing for multiple registrationsin 2.3.13 - really stlport with typetraits

[Malcolm Davey]

Hi Karel

I think I have finally resolved the issue - it's a combination of using
mico with stlport when type traits are enabled to do optimized copying
of items in std containers and algorithms.

(in mico 2.3.13 )
The problem occurs in poa_impl.cc
On the line 418
current->push_back (CurrentState (_poa, _por, _serv));

The previous sample code I sent with multiple calls to bind_new_context
will exercise the problem, because it results in multiple CurrentState
objects being added to the container in question.

I think there is a problem with stlport when it tries to optimize the
insertion in the push_back. It does a check to see if the class in
question (in our case CurrentState) has a trivial assignment operator -
which it does, and then optimizes in the case where the vector needs to
do a reallocation. When it is optimized it does a call to clear() hence
calling CurrentState's destructor for items already in the container
hence decreasing the reference count of serv.
It then does a copy assuming it can do a memcpy, skipping the actual
copy constructor and skipping the add_ref on the serv object.
This results in the serv object ultimately being deleted while it is
still in use.
It seems the problem is in stlport (at least as far back as 5.1.5 and in
the current one - 5.2). The test in the stlport code should be to
determine if there is a trival copy constructor NOT assignment operator.
To avoid the issue on mico's side, adding an assignment operator (with
an assert(0) like the default constructor) would avoid the issue - its
generally not common to have just. Making the default constructor or
assignment operator private and not define them might be an even better
option - I haven't test this yet and am not sure if it avoids the issue
with stlport.


(in stlport _vector.h)

void push_back(const _Tp& __x) {
if (this->_M_finish != this->_M_end_of_storage._M_data) {
_Copy_Construct(this->_M_finish, __x);
++this->_M_finish;
}
else {
typedef typename
__type_traits<_Tp>::has_trivial_assignment_operator _TrivialCopy;
_M_insert_overflow(this->_M_finish, __x, _TrivialCopy(), 1UL,
true);
}
}

This issue could happen with earlier versions of mico, but we had type
traits in stlport off initially because it caused mico to not compile.
Later we made a change to our stlport headers to enable mico to compile
with type traits. This change is in stlport 5.2. Some other OS's might
not enable type traits with stlport and hence might avoid the issue.

Malcolm

[Karel Gardas]

Hi Malcolm,

it's great that you finally nailed down the culprit. As you have
invested quite some time to it and as there are MICO users here and
there asking for STLport support, could you be so kind and test a change
where either you change default ctor of CurrentState to be private or
create "default" assignment operator with assert(0) as you describe? If
this goes well I'm in favor of private default ctor. If you are lucky
with the change and it runs fine on top of your STLport, please send me
the patch (or whole file) and I'll test it here and include (if it does
not break anything).

Thanks!
Karel

Post by Malcolm Davey
Hi Karel
I think I have finally resolved the issue - it's a combination of using
mico with stlport when type traits are enabled to do optimized copying
of items in std containers and algorithms.
(in mico 2.3.13 )
The problem occurs in poa_impl.cc
On the line 418
current->push_back (CurrentState (_poa, _por, _serv));
The previous sample code I sent with multiple calls to bind_new_context
will exercise the problem, because it results in multiple CurrentState
objects being added to the container in question.
I think there is a problem with stlport when it tries to optimize the
insertion in the push_back. It does a check to see if the class in
question (in our case CurrentState) has a trivial assignment operator -
which it does, and then optimizes in the case where the vector needs to
do a reallocation. When it is optimized it does a call to clear() hence
calling CurrentState's destructor for items already in the container
hence decreasing the reference count of serv.
It then does a copy assuming it can do a memcpy, skipping the actual
copy constructor and skipping the add_ref on the serv object.
This results in the serv object ultimately being deleted while it is
still in use.
It seems the problem is in stlport (at least as far back as 5.1.5 and in
the current one - 5.2). The test in the stlport code should be to
determine if there is a trival copy constructor NOT assignment operator.
To avoid the issue on mico's side, adding an assignment operator (with
an assert(0) like the default constructor) would avoid the issue - its
generally not common to have just. Making the default constructor or
assignment operator private and not define them might be an even better
option - I haven't test this yet and am not sure if it avoids the issue
with stlport.
(in stlport _vector.h)
void push_back(const _Tp& __x) {
if (this->_M_finish != this->_M_end_of_storage._M_data) {
_Copy_Construct(this->_M_finish, __x);
++this->_M_finish;
}
else {
typedef typename
__type_traits<_Tp>::has_trivial_assignment_operator _TrivialCopy;
_M_insert_overflow(this->_M_finish, __x, _TrivialCopy(), 1UL,
true);
}
}
This issue could happen with earlier versions of mico, but we had type
traits in stlport off initially because it caused mico to not compile.
Later we made a change to our stlport headers to enable mico to compile
with type traits. This change is in stlport 5.2. Some other OS's might
not enable type traits with stlport and hence might avoid the issue.
Malcolm

[Malcolm Davey]

Hi Karel

I have previously tried adding the assignment operator and this works.
I didn't try the changing the other calls to private, as this would not
make any difference, but this is an additional check which would be
good. The default constructor and assignment operator are not actual
called, it's just that type_trait intrinsics which Stlport detects the
existence of them in order to do further optimizations, hence the
privacy of them makes no difference.
Make these functions private would be good since calling them would
cause problems.

Malcolm

-----Original Message-----
From: Karel Gardas [mailto:***@objectsecurity.com]
Sent: Tuesday, 2 December 2008 6:06 AM
To: Malcolm Davey
Cc: mico-***@mico.org
Subject: Re: FW: [mico-devel] nsd registration failing for multiple
registrationsin 2.3.13 - really stlport with typetraits


Hi Malcolm,

it's great that you finally nailed down the culprit. As you have
invested quite some time to it and as there are MICO users here and
there asking for STLport support, could you be so kind and test a change
where either you change default ctor of CurrentState to be private or
create "default" assignment operator with assert(0) as you describe? If
this goes well I'm in favor of private default ctor. If you are lucky
with the change and it runs fine on top of your STLport, please send me
the patch (or whole file) and I'll test it here and include (if it does
not break anything).

Thanks!
Karel

Post by Malcolm Davey
Hi Karel
I think I have finally resolved the issue - it's a combination of using
mico with stlport when type traits are enabled to do optimized copying
of items in std containers and algorithms.
(in mico 2.3.13 )
The problem occurs in poa_impl.cc
On the line 418
current->push_back (CurrentState (_poa, _por, _serv));
The previous sample code I sent with multiple calls to

bind_new_context

Post by Malcolm Davey
will exercise the problem, because it results in multiple CurrentState
objects being added to the container in question.
I think there is a problem with stlport when it tries to optimize the
insertion in the push_back. It does a check to see if the class in
question (in our case CurrentState) has a trivial assignment operator -
which it does, and then optimizes in the case where the vector needs to
do a reallocation. When it is optimized it does a call to clear() hence
calling CurrentState's destructor for items already in the container
hence decreasing the reference count of serv.
It then does a copy assuming it can do a memcpy, skipping the actual
copy constructor and skipping the add_ref on the serv object.
This results in the serv object ultimately being deleted while it is
still in use.
It seems the problem is in stlport (at least as far back as 5.1.5 and in
the current one - 5.2). The test in the stlport code should be to
determine if there is a trival copy constructor NOT assignment

operator.

Post by Malcolm Davey
To avoid the issue on mico's side, adding an assignment operator (with
an assert(0) like the default constructor) would avoid the issue - its
generally not common to have just. Making the default constructor or
assignment operator private and not define them might be an even better
option - I haven't test this yet and am not sure if it avoids the issue
with stlport.
(in stlport _vector.h)
void push_back(const _Tp& __x) {
if (this->_M_finish != this->_M_end_of_storage._M_data) {
_Copy_Construct(this->_M_finish, __x);
++this->_M_finish;
}
else {
typedef typename
__type_traits<_Tp>::has_trivial_assignment_operator _TrivialCopy;
_M_insert_overflow(this->_M_finish, __x, _TrivialCopy(), 1UL,
true);
}
}
This issue could happen with earlier versions of mico, but we had type
traits in stlport off initially because it caused mico to not compile.
Later we made a change to our stlport headers to enable mico to compile
with type traits. This change is in stlport 5.2. Some other OS's might
not enable type traits with stlport and hence might avoid the issue.
Malcolm