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

Temporarily disable the lib-dynlink-domains test on Windows #11607

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Oct 6, 2022

Since turning AppVeyor back on, we've been seeing frequent failures in the lib-dynlink-domains test from #11032.

This test is correctly revealing that FlexDLL is not thread-safe. It's very easy to reproduce the problem - even if the test succeeds, running main.exe from the ocamlopt.byte test output directory a few times will reveal one or other of various corruptions! I've verified with a quickly hacked patch that fixing flexdll_wdlopen and flexdll_dlsym for multi-threaded access eliminates the crash. The lock used in bytecode to protect the Symtable machinery will be also protecting flexlink when this is executed in bytecode, so this is a native code problem (AppVeyor does run all the dynlink tests in native code for, um, this very reason apparently!)

This test fails more regularly than it passes, so until I've patched flexdll, I suggest we disable it on Windows.

Temporarily, until flexdll has been patched for parallel use of dlopen
and dlsym.
Copy link
Member

@Octachron Octachron left a comment

Choose a reason for hiding this comment

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

I agree that it is fine to temporarily disable a test due to a known and localized issue in a semi-external tool.

@dra27 dra27 merged commit 131fbf3 into ocaml:trunk Oct 6, 2022
dra27 added a commit that referenced this pull request Oct 6, 2022
Temporarily disable the lib-dynlink-domains test on Windows

(cherry picked from commit 131fbf3)
@dra27 dra27 deleted the disable-dynlink-domains-windows branch October 6, 2022 13:14
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.

None yet

2 participants