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

Move timer objects to the end of member list #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

unjello
Copy link

@unjello unjello commented Mar 12, 2018

In high CPU load it can happen, that Timer thread wakes up,
is swapped out, and when application resumes it does not resume with timer
thread, but transport thread, that decides to do failover, destroys
InactivityMonitor object. When its members are getting destroyed,
they arrive at Timer object that joins, timer thread resumes in
writeCheck or readCheck method, only to find out asyncTasks is already
destroyed because it comes later in member declaration list so it was
destroyed first. Moving Timer object to the end of the list, guarantees
that they will be destroyed first, so their threads will get joined and
allowed to finish, before the rest of the object is destroyed.

Closes AMQCPP-626

In high CPU load it can happen, that Timer thread wakes up,
is swapped out, and when application resumes it does not resume with timer
thread, but transport thread, that decides to do failover, destroys
`InactivityMonitor` object. When its members are getting destroyed,
they arrive at `Timer` object that joins, timer thread resumes in
`writeCheck` or `readCheck` method, only to find out `asyncTasks` is already
destroyed because it comes later in member declaration list so it was
destroyed first. Moving `Timer` object to the end of the list, guarantees
that they will be destroyed first, so their threads will get joined and
allowed to finish, before the rest of the object is destroyed.

Closes AMQCPP-626
@unjello
Copy link
Author

unjello commented Mar 28, 2018

Guys. Is there anything that needs to happen, so this PR gets some (any) attention?

@Pingmin
Copy link

Pingmin commented Mar 31, 2018

Just wait and keep patient, or maybe you would like to send an email to the master.

@unjello
Copy link
Author

unjello commented Mar 31, 2018

Needing to wait almost three weeks for a patch for a serious, crash-causing, race-condition to receive any attention, doesn't exactly build trust in a library. I have sent 2 emails to dev list, as well as logged JIRA. Not sure what (who?) the "master" is, but I feel like I've done enough. Thank you for the tip tho. I'll just wait and keep being patient.

@Pingmin
Copy link

Pingmin commented Apr 1, 2018

@tabish121

@unjello
Copy link
Author

unjello commented Apr 12, 2018

@Pingmin thank you for trying, but after one month with no reaction whatsoever, and more than a year since last release, I should probably come to a conclusion that this project is effectively abandoned.

@Pingmin
Copy link

Pingmin commented Apr 15, 2018

@unjello I don't think so and we don't want to see that. Maybe He is too busy in these months...

@unjello
Copy link
Author

unjello commented Feb 23, 2024

Many years later. Any news?

@sandym
Copy link

sandym commented Feb 23, 2024

Good luck @unjello ! I was about to do a PR to update zlib and get rid of a CVE alert we have. I don't think I'll waste time with that.

@mattrpav
Copy link

Hi @unjello @sandym

I'm an ActiveMQ committer and would be able to help modernize. My C++ is a bit rusty, so I'd need an assist. I think first step would be to modernize the tool chain (add git action and jenkins file to get some automated build testing going.

Would either of you be able to assist in PRs for that?

@arayq2
Copy link

arayq2 commented Feb 25, 2025

The patch code is incomplete. The initialization list in the constructor needs modification as well.
https://isocpp.org/wiki/faq/ctors#ctor-initializer-order

@unjello
Copy link
Author

unjello commented Feb 25, 2025

Hi, don’t want to come across rude, but 7 years later I barely use C++ anymore and don’t have any use for AMQ either. I won’t have time to assist with this, sorry.

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.

5 participants