Skip to content

[FIX] queue_job: job runner open pipe that are never closed properly - #754

Merged
OCA-git-bot merged 1 commit into
OCA:14.0from
petrus-v:14.0-fix-fd-open
Mar 19, 2025
Merged

[FIX] queue_job: job runner open pipe that are never closed properly#754
OCA-git-bot merged 1 commit into
OCA:14.0from
petrus-v:14.0-fix-fd-open

Conversation

@petrus-v

Copy link
Copy Markdown
Contributor

this make a lot of open file descriptors that we got the limit while instentiate jobrunner from queue_job_cron_jobrunner in the current channel implementation #750

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @guewen,
some modules you are maintaining are being modified, check this out!

@petrus-v
petrus-v marked this pull request as ready for review March 14, 2025 15:37
this make a lot of open file descriptors that we got the limit while instentiate
jobrunner from queue_job_cron_jobrunner in the current channel implementation
OCA#750

@amh-mwamh-mw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code review only, LGTM. If this gets forward-ported to 15 or 18, I would be willing to graph file descriptor counts in one of my long running test environments and report back.

@petrus-v

Copy link
Copy Markdown
ContributorAuthor

@amh-mw Thanks for review

Code review only, LGTM. If this gets forward-ported to 15 or 18, I would be willing to graph file descriptor counts in one of my long running test environments and report back.

I don't expect you get that mush open fd using queue_job as it. We discover it by using queue_job_cron_jobrunner and this recent PR #750 where a QueueJobRunner instance is created while acquiring job so we get the limit while handling thousands of jobs quickly. Indeed this is a bit mitigate as workers are restart at some point.

Anyway it's an esay way to mitigate a denial of service attack (assuming attacker is able to execute such code from odoo.addons.queue_job.jobrunner import QueueJobRunner; [QueueJobRunner.from_environ_or_config() for _ in range(2048)] ... well I suppose if an attacker is able to do that it can do more dommage on the server ! ).

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@simahawk

Copy link
Copy Markdown
Contributor

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-754-by-simahawk-bump-patch, awaiting test results.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 1840d7c. Thanks a lot for contributing to OCA. ❤️

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@petrus-v@OCA-git-bot@simahawk@amh-mw@damdam-s