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 #10574 - environment pointer sometimes changes before exec, causing error in child process. #10575

Merged
merged 1 commit into from
Dec 7, 2024

Conversation

schveiguy
Copy link
Member

This is a nasty problem. This sort of works around the issue by assuming the environment is as expected in the child process.

If you want to merge parent + custom environment, there is still a risk of segfault here, as another thread can easily change the environment pointer and cause the pointer being iterated to be invalidated.

But that is a separate problem that has no good solution. See discussions on the Julia lang github: JuliaLang/julia#34726

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @schveiguy!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

  • In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + phobos#10575"

@schveiguy
Copy link
Member Author

How do we get the bot to be happy with the bug reference? @burner

@jmdavis
Copy link
Member

jmdavis commented Dec 5, 2024

How do we get the bot to be happy with the bug reference? @burner

Well, the bot was changed at some point so that you had to mention bugzilla specifically at the front of the commit message - e.g. "fix Bugzilla Issue 10574". Maybe "fix Github Issue 10574" would work? Though of course it's always possible that that part was forgotten and hasn't been implemented yet.

@schveiguy
Copy link
Member Author

Huh, there is some issue with public unittest failures?

@jmdavis
Copy link
Member

jmdavis commented Dec 5, 2024

Huh, there is some issue with public unittest failures?

Nick reported the same problem in slack yesterday. I have no clue how we could have gotten in such a state though given that the test suite should have caught it.

@thewilsonator
Copy link
Contributor

@burner
Copy link
Member

burner commented Dec 5, 2024

Huh, there is some issue with public unittest failures?

dlang/dlang-bot#305

@kinke
Copy link
Contributor

kinke commented Dec 6, 2024

Huh, there is some issue with public unittest failures?

Nick reported the same problem in slack yesterday. I have no clue how we could have gotten in such a state though given that the test suite should have caught it.

AFAICT, these extra tests are only run for some CI jobs (Circle and Buildkite) here in Phobos. I suspect the problem was introduced in the compiler in dlang/dmd#15343 which changed the lambda mangles (where CI apparently didn't run these public Phobos unittests). I've seen similar undefined-lambda-symbols for a Symmetry codebase with LDC v1.40.0-beta5, which includes that mangling change.

Edit: Trying to include these tests for DMD CI: dlang/dmd#17107

@kinke
Copy link
Contributor

kinke commented Dec 7, 2024

CI should be fixed now; please rebase or dummy-amend to retrigger CI.

@dlang-bot dlang-bot merged commit d767448 into dlang:stable Dec 7, 2024
9 checks passed
@schveiguy schveiguy deleted the fix10574 branch December 7, 2024 22:02
Sign up for free to 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.

6 participants