Seg Fault of reader deconstruction

8 posts / 0 new
Last post
Offline
Last seen: 1 month 1 week ago
Joined: 03/31/2022
Posts: 11
Seg Fault of reader deconstruction

Folks,

I have a new problem. I have a class with a short lifespan. It creates a reader in its constructor, and deletes it automatically in the destructor. The class itself acts as the listener:

template<class Topic> PushReader : public dds::sub::NoOpDataReaderListener<Topic>
{
 
    public :
        PushReader();
        virtual ~PushReader();
    private :
        dds::sub::DataReader<Topic> _reader;
 
        void on_data_available(dds::sub::DataReader<Topic> &reader);
}
 
template<class Topic> PushReader<Topic>::PushReader() : _reader(dds::core::null)
{
    // Create _reader and set this as the listener
}
 
template<class Topic> PushReader<Topic>::~PushReader()
{
    if( _reader != dds::core::null )
    {
        if( _reader.get_listener() != nullptr )
        {
            _reader.set_listener(nullptr);
        }
    }
}
 

The class I use is derived from PushReader. When it is deleted, I get a Seg Fault from the set_listner line. If I comment out the set_listener line, it Seg Faults at the end of the destructor.

So the question is : What am I doing wrong for this to happen ? According to the set_listener documentation setting it to null is how a listener is removed.

Dale Pennington

 

 

 

 

 

 

 

 

Howard's picture
Offline
Last seen: 3 hours 54 min ago
Joined: 11/29/2012
Posts: 381

In what thread are you deleting the PushReader object?

Offline
Last seen: 1 month 1 week ago
Joined: 03/31/2022
Posts: 11

It is in the main thread, same thread where it is created.

Basically I am creating an object in the main thread, which in turn creates the PushReader based object, works a little with it, then deletes it.

 

Dale

Offline
Last seen: 2 weeks 14 hours ago
Joined: 04/02/2013
Posts: 179

How are you setting the listener? Make sure you're not doing this:

shared_ptr<>(this)

(See https://en.cppreference.com/w/cpp/memory/enable_shared_from_this for an explanation why that will crash).

I would recommend to make the listener a separate class. You can define it inside PushReader to keep it hidden.

 

Offline
Last seen: 1 month 1 week ago
Joined: 03/31/2022
Posts: 11

<edit correction> I am using a shared point, as that is the example and required by the interface. I cannot just do set_listener(this)

I could certainly do the other approach, but that looked like much more complicated code to pass along. Especially given the levels of inheritance I am using. And at some point we have to pass from PushReader back to the caller.

I am putting together a quick test program to do create/.destroy to see if I can recreate in a simple environment.

Dale

Howard's picture
Offline
Last seen: 3 hours 54 min ago
Joined: 11/29/2012
Posts: 381

Yes, you need to pass a shared pointer to the "set_listener()" method.  But as Alex wrote, you can't do that with a "this" pointer in the way that you're doing it...it's not a DDS issue, it's how C++ works.

Here's a file (see attachment) that shows how to do it with a internal Listener class inside of PushReader.  I've compiled and tested and it works (with deletion w/o segfault).

 

 

File Attachments: 
Howard's picture
Offline
Last seen: 3 hours 54 min ago
Joined: 11/29/2012
Posts: 381

Here's another version in which the internal Listener class calls PushReader::on_data_available()...should you want the parent class to actually implement the callback.

File Attachments: 
Offline
Last seen: 1 month 1 week ago
Joined: 03/31/2022
Posts: 11

Thanks for the feedback. That helps clear things up

Dale