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

fix memory leak in jeedom_com & small code quality improvment #69

Merged
merged 4 commits into from
Dec 26, 2024

Conversation

Mips2648
Copy link
Contributor

@Mips2648 Mips2648 commented Dec 19, 2024

Description

This PR aim to fix the memory leak observed on deb11. I cannot explain why it didn't happen on others versions.
It was due to the fact that the "send async thread" was creating new thread for each new cycle (from 0.3s for most of the plugin to 30s for some like plugin-sms); and that's why the issue had small of huge impact: it depends the plugin cycle config.

So "send async thread 1" creates "send async thread 2" which will create "send async thread 3" etc; meaning that potentially none of this thread ever stop until the very last one stops which happen only when daemon is stopped; so the daemon continuously create new thread and which consume additional memories.

The correct way to do this is to have one and only one thread which will run forever until the daemon stop. To make sure the thread is stopped when the parent process (the daemon) is stopped, we flag that thread as a "daemon thread" = never-ending background task.

I took the opportunity to clean a bit the rest of the code as well.

===

to fix issue in each plugin, in theory we just need to copy/past the new version of jeedom.py to the plugin BUT we should first check if there wasn't any local changes made

===

Suggested changelog entry

None.

Related issues/external references

Fixes #

Types of changes

  • Bug fix (non-breaking change which fixes)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the GNU.
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added MD documentation for the sniff.

@Mips2648 Mips2648 requested review from zoic21 and Salvialf December 19, 2024 15:55
@Mips2648
Copy link
Contributor Author

fix confirmed on at least 2 plugins (broadlink & rfplayer2), I'll merge this

@Mips2648 Mips2648 merged commit 6a35720 into jeedom:master Dec 26, 2024
4 checks passed
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.

2 participants