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 CI errors and unittest compile error on certain compiler versions #411

Merged
merged 5 commits into from
Sep 11, 2024

Conversation

s-ludwig
Copy link
Member

Affects the CI of the vibe.d repository

Ensures that the close operation has finished before returning from the destructor in order to ensure the owning thread doesn't terminate prematurely, resulting in a crash. This fixes Windows CI failures.
@Geod24
Copy link
Contributor

Geod24 commented Sep 10, 2024

What's the in compiler bug ?

@Geod24
Copy link
Contributor

Geod24 commented Sep 10, 2024

If you can provide me a command line example of compilation and the error message, I can advise.
The obvious reason why you would get a linker error with in is if you are compiling a library with a different set of flag than what uses it (with one of those compiling with -preview=in but not the other). In which case it would be a feature, not a bug, as the alternative is memory corruption ;)

@s-ludwig
Copy link
Member Author

I think it was with dub-registry where I had consistent unresolved symbol errors for all methods that used simple in parameters (both, DMD 2.109.1 and LDC 1.39.0). All is compiled in a single dub build run, so there should be no compiler argument differences and -preview=in shouldn't be involved, unless someone else sneaked it in somewhere. I didn't look for, or write, a bug report so far, because I'm still in a kind of clean-up process after upgrading MongoDB, which caused a little cascade of issues.

@Geod24
Copy link
Contributor

Geod24 commented Sep 10, 2024

@s-ludwig
Copy link
Member Author

Okay, I see. I had a quick look, but missed the obvious possibility of dub. Would moving those into the "application" configuration do any harm?

Avoids relying on closing the stream through RAII, which acts worse in terms of error reporting in the presence of in-flight exceptions.
@s-ludwig s-ludwig changed the title Fix unittest compile error on certain compiler versions Fix CI errors and unittest compile error on certain compiler versions Sep 10, 2024
s-ludwig added a commit to dlang/dub that referenced this pull request Sep 10, 2024
Otherwise leads to linker errors when building together with other code that uses `in` parameters. See also vibe-d/vibe-core#411
@l-kramer l-kramer merged commit 6bbcb27 into master Sep 11, 2024
13 checks passed
@l-kramer l-kramer deleted the fix_unittest_error branch September 11, 2024 10:01
dlang-bot pushed a commit to dlang/dub that referenced this pull request Sep 11, 2024
Otherwise leads to linker errors when building together with other code that uses `in` parameters. See also vibe-d/vibe-core#411
s-ludwig added a commit that referenced this pull request Sep 13, 2024
Closing of the file stream must only be done when the last reference gets destroyed.

Note that this requires an API addition in eventcore, which is why the code has been wrapped in a static if.
l-kramer added a commit that referenced this pull request Sep 13, 2024
Fix a regression introduced by #411
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.

3 participants