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

Replace 'this' with shared_from_this to prevent shutdown issues #559

Closed
wants to merge 1 commit into from

Conversation

kheaactua
Copy link
Contributor

@kheaactua kheaactua commented Nov 3, 2023

This change appears to reduce but not entirely eliminate crashes on SIGTERM.

The key signature of the crash appears to be:

#34 vsomeip_v3::routing_manager_impl::expire_subscriptions (this=0x1962493058, _force=false) at implementation/routing/src/routing_manager_impl.cpp:3307 #35 0x0000004ec2948f2c in vsomeip_v3::sd::service_discovery_impl::expire_subscriptions (_error=..., this=0x19624985c8) at implementation/service_discovery/src/service_discovery_impl.cpp:2828

@kheaactua
Copy link
Contributor Author

I don't know is this PR is comprehensize enough, I would really appreciate Covesa's opinion on it.

AOS-141070

This change appears to reduce but not entirely eliminate crashes on
SIGTERM.

The key signature of the crash appears to be:
COVESA#34 vsomeip_v3::routing_manager_impl::expire_subscriptions (this=0x1962493058, _force=false) at implementation/routing/src/routing_manager_impl.cpp:3307
COVESA#35 0x0000004ec2948f2c in vsomeip_v3::sd::service_discovery_impl::expire_subscriptions (_error=..., this=0x19624985c8) at implementation/service_discovery/src/service_discovery_impl.cpp:2828
@kheaactua kheaactua marked this pull request as draft November 3, 2023 17:11
@kheaactua kheaactua marked this pull request as ready for review November 4, 2023 14:27
@vvcarvalho
Copy link

I'm not sure about this change, given we are inverting the lifetime of the parent object (service_discovery) and the child object (timers). If the timers are causing some of the crashes, it may be better to handle stopping the timers on shutdown or even put it into the destructor, instead of extending the lifetime of service_discovery data.

Copy link
Contributor

@goncaloalmeida goncaloalmeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check @vvcarvalho comment.

@kheaactua
Copy link
Contributor Author

I'm not sure about this change, given we are inverting the lifetime of the parent object (service_discovery) and the child object (timers). If the timers are causing some of the crashes, it may be better to handle stopping the timers on shutdown or even put it into the destructor, instead of extending the lifetime of service_discovery data.

I can give that a try. I'm not yet up and running with 3.4.9-r1 on QNX (still migrating to our 7.1 with io-pkt), but I have 3.3.8 running well.

@kheaactua
Copy link
Contributor Author

(I haven't forgot, just haven't been able to get to this yet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants