Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[15446] TypeObject for sequences/arrays/maps of nested types crashes #3296

Closed
1 task done
JLBuenoLopez opened this issue Feb 14, 2023 · 6 comments
Closed
1 task done
Labels
need more info Issue that requires more info from contributor

Comments

@JLBuenoLopez
Copy link
Contributor

JLBuenoLopez commented Feb 14, 2023

Is there an already existing issue for this?

  • I have searched the existing issues

Expected behavior

Registering a TypeObject with a sequence/array/map with nested types is not supposed to crash.

Current behavior

General bug description

Registering a TypeObject coming from an IDL defining a sequence/array of a nested structure causes a crash.
This is caused when the TypeObject is only registered in one of the applications and the information is published with the DomainParticipant discovery information, DATA(p), and the remote application tries registering the TypeObject.
A bug in TypeObjectFactory::is_type_identifier_complete causes the crash.
This method is called recursively for nested structures.
The nested structure has not been properly registered so the element_identifier is nullptr.
TypeObjectFactory::is_type_identifier_complete does not have a safety check to ensure the validity of the given pointer.

On the one hand, a patch adding this safety check ensuring that Fast DDS does not crash would be good to have.
On the other hand, registering a sequence/array/map with nested types should succeed.
This second point requires further investigation.

Workaround

This issue can be prevented avoiding publishing the TypeObject information in the DomainParticipant discovery information.
This can be configured within the TopicDataType class with members auto_fill_type_object(false) and auto_fill_type_information(false).

Manifestations

This bug has several manifestations:

Dynamic types

Using a TypeObject that defines sequences/arrays/maps of nested types will fail unless the TypeObject is registered in every DomainParticipant (which is contrary to Dynamic types philosophy).
Consequently, this bug prevents the use of Dynamic types with these specific types.

Content Filtered Topics

In order to use the default filter class requires registering the TypeObject.
Consequently, this issue might appear.

Related issues

Steps to reproduce

The following IDL file can be used to generate the conflicting TypeObject using Fast DDS-Gen with -typeobject option enabled.

struct HelloWorld
{
    sequence<string, 128> messages;
};

struct HelloWorldList
{
    sequence<HelloWorld, 250> helloworld_list;
};

Registering the TypeObject in one DomainParticipant and launching another DomainParticipant with only the DataType registered (not the TypeObject) will cause the crash of this second DomainParticipant.

Fast DDS version/commit

This bug was first reported in Fast DDS v2.3.x (#2184) and is still applying to every supported branch in Fast DDS, including master.

Platform/Architecture

Ubuntu Focal 20.04 amd64, Ubuntu Focal 20.04 arm64, Windows 10 Visual Studio 2019, MacOS Mojave 10.14

Transport layer

UDPv4, UDPv6, TCPv4, TCPv6, Shared Memory Transport (SHM), Intra-process, Data-sharing delivery, Zero copy

Additional context

No response

XML configuration file

No response

Relevant log output

No response

Network traffic capture

No response

@JLBuenoLopez JLBuenoLopez added the bug Issue to report a bug label Feb 14, 2023
@JLBuenoLopez JLBuenoLopez changed the title TypeObject for sequences/arrays/maps of nested types crashes [15446] TypeObject for sequences/arrays/maps of nested types crashes Feb 14, 2023
@methylDragon
Copy link
Contributor

methylDragon commented Mar 24, 2023

@JLBuenoLopez-eProsima this other issue might be related: #2970 , since it seems there's an issue with default array values?

It causes issues with printing too. I think there's a problem with default array values being deleted when they're not supposed to be.

@JLBuenoLopez
Copy link
Contributor Author

@methylDragon, Dynamic types feature is being refactored right now (in #3392). It is expected to be merged in the coming months. Sorry for the delay.

@JLBuenoLopez JLBuenoLopez linked a pull request Apr 14, 2023 that will close this issue
13 tasks
@AndreaFinazzi
Copy link

AndreaFinazzi commented Aug 2, 2023

I'm facing the same issue without any (apparent) nested type.

Type definition:

module my_package { module msg {
struct Timestamp {
    unsigned long long nanoseconds;
    unsigned long long seconds;
}; }; };

module my_package { module msg {
struct CarControl {
    my_package::msg::Timestamp timestamp;
    float steering_target;
}; }; };

module my_package { module msg {
        @final struct ControlsTrajectory {
            my_package::msg::Timestamp timestamp;  
            sequence<my_package::msg::CarControl> controls;
}; }; }; 

version/commit

Source code generated with Fast-DDS-Gen:

  • version: v2.5.1
  • options: -typeros2 -typeobject -replace -traits -cs

Fast-DDS:

  • version: fast-dds/2.10.1 (conancenter)

Manifestation

In the call to the my_package::msg::ControlsTrajectory constructor, I get a segfault at the line

cst_controls.common().member_type_id(*TypeObjectFactory::get_instance()->get_sequence_identifier("my_package::msg::CarControl", 100, true));

of ControlsTrajectoryTypeObject.cxx, as the call to get_sequence_identifier returns an invalid pointer.

EDIT:

I noticed that in TypeObjectFactory::get_instance()->complete_identifiers_ there's an instance of my_package::msg::dds_::CarControl_. This might point to a different kind of issue.

EDIT (2):

With Fast-DDS-Gen on eProsima/Fast-DDS-Gen#160 I was able to avoid the abovementioned segfault (TypeObjectFactory::get_instance()->complete_identifiers_ has both my_package::msg::dds_::CarControl_ and my_package::msg::CarControl).
The manifestation is now equivalent to the one explained in this issue, the program crashes on

bool TypeObjectFactory::is_type_identifier_complete(
        const TypeIdentifier* identifier) const
{
    switch (identifier->_d())
    {
        // ...
        case TI_PLAIN_SEQUENCE_SMALL:
            return is_type_identifier_complete(identifier->seq_sdefn().element_identifier());

as identifier->seq_sdefn().element_identifier() returns nullptr.

@JesusPoderoso
Copy link
Contributor

Hi @methylDragon @gDorndorf @fwr-ml @AndreaFinazzi @doruktiktiklar, Fast DDS v3.0.0 has been released, including the mentioned Dynamic Types refactor. Could you check if the reported problems persist?

@JesusPoderoso JesusPoderoso added need more info Issue that requires more info from contributor and removed bug Issue to report a bug labels Aug 29, 2024
@fwr-ml
Copy link

fwr-ml commented Aug 30, 2024

@JesusPoderoso I'd love to, but lack the capacity, for now. Happy, if you've sorted it out, though.

(Also, a PR of yours - #2207 I think - did the trick for me. My software doesn't learn DynamicType s remotely, so I think, I wouldn't test your changes anyway.
And I work with a heavily patched fast-dds-gen - so I guess, I'll have a lot of work to do to catch up. One day, but not soon.)

@Mario-DL
Copy link
Member

Mario-DL commented Sep 5, 2024

As the Dynamic Types refactor has been already introduced in the latest Fast DDS v3.0.0, we internally agreed to close all Dynamic-Types related issues.
We elaborated a Migration guide to help with the upgrade process to this new major release.

Please, feel free to reopen it if the issue persists.

@Mario-DL Mario-DL closed this as completed Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more info Issue that requires more info from contributor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants