Skip to content

✨Remove unsafe rust code#2613

Open
eselimsen wants to merge 3 commits intomainfrom
feat/meta-7111-remove-unsafe/main
Open

✨Remove unsafe rust code#2613
eselimsen wants to merge 3 commits intomainfrom
feat/meta-7111-remove-unsafe/main

Conversation

@eselimsen
Copy link
Copy Markdown

eselimsen added 2 commits May 4, 2026 23:26
Removes unsafe rust usages from windmill and forbids it in cargo config.
Unsafe static accesses are replaced with std::RwLock as those were global settings written only once or a few times, but read often.
Copilot AI review requested due to automatic review settings May 5, 2026 09:32
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement (CLA).

To sign the CLA, please comment on this PR with:
I have read the CLA Document and I hereby sign the CLA

This is a one-time requirement. Once you have signed, all future contributions will be covered.

You can read the full CLA document here: https://github.com/sequentech/step/blob/main/.github/cla/CLA.md


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces static mut globals in the Windmill Celery service with lock-based shared state and adds a crate-level lint to forbid unsafe Rust, aligning with the goal of removing unsafe code from this binary.

Changes:

  • Reworked global Celery settings/state in celery_app.rs to use LazyLock + RwLock instead of static mut.
  • Updated main.rs to use the new configuration API (set_config, scoped imports, error propagation from semaphore init).
  • Added unsafe_code = "forbid" in Cargo.toml and applied formatting cleanups.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/windmill/src/services/celery_app.rs Replaces unsafe global mutable state with lock-protected config/state and adjusts Celery app initialization.
packages/windmill/src/bin/main.rs Migrates startup/config wiring to the new Celery configuration API.
packages/windmill/Cargo.toml Adds crate-level unsafe-code lint and formatting-only dependency edits.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +229 to +238
let CeleryConfig {
prefetch_count,
acks_late,
task_max_retries,
broker_connection_max_retries,
heartbeat_secs,
..
} = *CELERY_CONFIG
.read()
.map_err(|_| anyhow!("failed to read-lock CeleryConfig"))?;
@eselimsen
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

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