[omniORB] Problem with any.to_any in omniORBpy and DynAny in libomniDynamic

[Michael]

Hello,

this was a more complicated one, so I'm giving you some background:

We run a any serialization service with a signature like:

string anyToXml(in any a) raises (x,y,z);

We had issues when serializing certain dynamically created structures
from python, which resulted (based on the machine used) in Segfaults or
infinite hangs (but no CPU usage).

I could track down the problem to the following case in python:
serializer.anyToXml(any.to_any({'a':None}))

After inspecting what's going on, I figured that to_any does create
sequences and structures containing members of type TC_null (which IMHO
should not happen, instead they should put in an any "containing" TC_null).

I wrote a little test script for python:

from omniORB import any
import CORBA

print "Sequence test 1 (any.to_any([None, \"123\"]):"
print any.to_any([None, "123"])

print "\nSequence test 2 (any.to_any([None]):"
print any.to_any([None])

print "\nSequence test 3 (any.to_any([None,None]):"
print any.to_any([None,None])

print "\nStruct test1:"
print any.to_any({'a':None})
print "\nStruct test2:"
print any.to_any({'a':CORBA.Any(CORBA.TC_null, None)})

print "\nDirect None Test1:"
print any.to_any(CORBA.Any(CORBA.TC_null, None))

print "\nDirect None Test2:"
print any.to_any(None)

The output of this script is:

Sequence test 1 (any.to_any([None, "123"]):
CORBA.Any(orb.create_sequence_tc(bound=0, element_type=CORBA.TC_any),
[CORBA.Any(CORBA.TC_null, None), CORBA.Any(CORBA.TC_string, '123')])

Sequence test 2 (any.to_any([None]):
CORBA.Any(orb.create_sequence_tc(bound=0, element_type=CORBA.TC_null),
[None])

Sequence test 3 (any.to_any([None,None]):
CORBA.Any(orb.create_sequence_tc(bound=0, element_type=CORBA.TC_null),
[None, None])

Struct test1:
CORBA.Any(CORBA.TypeCode("omni:3cc98447:00000001"),
UnknownStruct<omni:3cc98447:00000001>(a=None))

Struct test2:
CORBA.Any(CORBA.TypeCode("omni:3cc98447:00000002"),
UnknownStruct<omni:3cc98447:00000002>(a=None))

Direct None Test1:
CORBA.Any(CORBA.TC_null, None)

Direct None Test2:
CORBA.Any(CORBA.TC_null, None)

IMHO the only valid tests are Direct Test 1+2 and Sequence test 1. I
digged through any.py and came up with the following (maybe not very
elegant) patch - I also attached it as any.py.patch:

--- any.py.orig Tue Aug 19 16:26:45 2008
+++ any.py Tue Aug 19 17:24:59 2008
@@ -210,7 +210,7 @@

atc = any_list[0]._t
for a in any_list:
- if not a._t.equivalent(atc):
+ if not a._t.equivalent(atc) or a._t.kind() == CORBA.tk_null:
break
else:
tc = tcInternal.createTypeCode((tcInternal.tv_sequence,
atc._d, 0))
@@ -238,8 +238,12 @@
t, v = _to_tc_value(v)
ms.append(k)
dl.append(k)
- dl.append(t._d)
- svals.append(v)
+ if t.kind() == CORBA.tk_null:
+ dl.append(CORBA.TC_any._d)
+ svals.append(CORBA.Any(CORBA.TC_null, None))
+ else:
+ dl.append(t._d)
+ svals.append(v)
cls = omniORB.createUnknownStruct(id, ms)
dl[1] = cls
tc = tcInternal.createTypeCode(tuple(dl))

The output of test script after applying the patch is:
Sequence test 1 (any.to_any([None, "123"]):
CORBA.Any(orb.create_sequence_tc(bound=0, element_type=CORBA.TC_any),
[CORBA.Any(CORBA.TC_null, None), CORBA.Any(CORBA.TC_string, '123')])

Sequence test 2 (any.to_any([None]):
CORBA.Any(orb.create_sequence_tc(bound=0, element_type=CORBA.TC_any),
[CORBA.Any(CORBA.TC_null, None)])

Sequence test 3 (any.to_any([None,None]):
CORBA.Any(orb.create_sequence_tc(bound=0, element_type=CORBA.TC_any),
[CORBA.Any(CORBA.TC_null, None), CORBA.Any(CORBA.TC_null, None)])

Struct test1:
CORBA.Any(CORBA.TypeCode("omni:2c074c84:00000001"),
UnknownStruct<omni:2c074c84:00000001>(a=CORBA.Any(CORBA.TC_null, None)))

Struct test2:
CORBA.Any(CORBA.TypeCode("omni:2c074c84:00000002"),
UnknownStruct<omni:2c074c84:00000002>(a=CORBA.Any(CORBA.TC_null, None)))

Direct None Test1:
CORBA.Any(CORBA.TC_null, None)

Direct None Test2:
CORBA.Any(CORBA.TC_null, None)

Which IMHO is the way it should work.
I'm not sure if there are other data structures affected, but afaik
struct and sequence are the only types created dynamically by any.to_any.

Now for the second issue: Why does the server side crash? (It only
crashes for structs, so I will limit my analysis on those):

The order of events is (all happening in dynAny.cc):
1. call to factory->create_dyn_any which calls
2. factory_create_dyn_any which calls
3. internal_create_any which creates
4. DynStructImpl::DynStructImpl (ctor) which calls
setNumComponents(actualTc()->NP_member_count());
5. DynAnyConstrBase::setNumComponents will create a list of components
and pupulate them by calling internal_create_dyn_any (this happens at
line 2430)
6. internal_create_dyn_any does not handle tk_null but
throws DynamicAny::DynAny::TypeMismatch();
7. This exception is not caught in the calling place, which means
that pd_components will never be initialized correctly
8. Segmentation fault (couldn't track down the exact position, but since
pd_components is filled with null pointers there are many places
where this might happen). Finding the exact point was beyond my time
constraints.

I would suggest to add exception handling in
DynAnyConstrBase::setNumComponents or do more rigid testing when
receiving anys (contained structs or sequences should not be allowed to
have members of kind tk_null).

cheers
Michael

-------------- next part --------------
--- any.py.orig Tue Aug 19 16:26:45 2008
+++ any.py Tue Aug 19 17:24:59 2008
@@ -210,7 +210,7 @@

atc = any_list[0]._t
for a in any_list:
- if not a._t.equivalent(atc):
+ if not a._t.equivalent(atc) or a._t.kind() == CORBA.tk_null:
break
else:
tc = tcInternal.createTypeCode((tcInternal.tv_sequence, atc._d, 0))
@@ -238,8 +238,12 @@
t, v = _to_tc_value(v)
ms.append(k)
dl.append(k)
- dl.append(t._d)
- svals.append(v)
+ if t.kind() == CORBA.tk_null:
+ dl.append(CORBA.TC_any._d)
+ svals.append(CORBA.Any(CORBA.TC_null, None))
+ else:
+ dl.append(t._d)
+ svals.append(v)
cls = omniORB.createUnknownStruct(id, ms)
dl[1] = cls
tc = tcInternal.createTypeCode(tuple(dl))

[Duncan Grisby]

On Tuesday 19 August, Michael wrote:

[...]

Post by Michael
serializer.anyToXml(any.to_any({'a':None}))
After inspecting what's going on, I figured that to_any does create
sequences and structures containing members of type TC_null (which
IMHO should not happen, instead they should put in an any "containing"
TC_null).

I'm not sure it's so wrong to have members of type TC_null. It's a bit
odd, but I don't think there's anything that makes it explicitly
illegal. I have Python code that makes use of sequences of nulls. I'm
therefore tempted to leave any.to_any as it is...

[...]

Post by Michael
Now for the second issue: Why does the server side crash? (It only

That, however, is definitely a bug. There were two related problems.
First, it didn't support structs / sequences of nulls (which I think it
should -- it already supported void). Second, when it threw the
exception to complain about the typecode, it corrupted itself and blew
up.

I've fixed both issues in CVS.

Does that change address your problem?

Cheers,

Duncan.

[Michael]

Hi Duncan,

thanks for your effort.
I've got two issues with the to_any code.

1. In the current implementation there is no way to pass an any
containing TC_null as part of a struct/sequence/union (even if it's
stated explicitely). In our scenario we actually need the possibility to
send "empty" anys which would make using to_any impossible.

2. The Corba Standard (corba2.6_01-12-01.pdf), section
"4.11.3 Creating TypeCodes" (page 4-59 at the bottom), states:

....
"Operations that take content or member types as arguments shall
check that they are legitimate (i.e., that they don't have kinds
tk_null, tk_void or tk_exception). If not, they shall raise the
BAD_TYPECODE exception with standard minor code 2. Operations that take
members shall check that the member"
....
(also available here:
http://www.cs.uwaterloo.ca/~mnojoumi/ThesisFiles/FinalSpec/CORBA/4.11.3.html)

So I think that besides from being strange this actually violates the
standard (that makes also sense from an IR/IDL perspective, because
there is no way to create such constructs through an IDL definition).

What do you think?

cheers
michael

Post by Duncan Grisby
[...]

Post by Michael
serializer.anyToXml(any.to_any({'a':None}))
After inspecting what's going on, I figured that to_any does create
sequences and structures containing members of type TC_null (which
IMHO should not happen, instead they should put in an any "containing"
TC_null).

I'm not sure it's so wrong to have members of type TC_null. It's a bit
odd, but I don't think there's anything that makes it explicitly
illegal. I have Python code that makes use of sequences of nulls. I'm
therefore tempted to leave any.to_any as it is...
[...]

Post by Michael
Now for the second issue: Why does the server side crash? (It only

That, however, is definitely a bug. There were two related problems.
First, it didn't support structs / sequences of nulls (which I think it
should -- it already supported void). Second, when it threw the
exception to complain about the typecode, it corrupted itself and blew
up.
I've fixed both issues in CVS.
Does that change address your problem?
Cheers,
Duncan.

[Duncan Grisby]

On Tuesday 16 September, Michael wrote:

[...]

Post by Michael
So I think that besides from being strange this actually violates the
standard (that makes also sense from an IR/IDL perspective, because
there is no way to create such constructs through an IDL definition).

OK, you've convinced me. I've modified to_any to always represent None
as an Any with null TypeCode, rather than a member of type null.

Cheers,

Duncan.