Repository navigation
Random crashes in System.Net.Mail test suites with mono interpreter #87271
Description
Description
This happens often on CI:
https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-heads-main-f9cccf963c964959af/System.Net.Mail.Functional.Tests/1/console.a97473cd.log?helixlogtype=result
To reproduce:
while true; do MONO_ENV_OPTIONS=--interp ./dotnet.sh build /t:Test /p:Configuration=Release src/libraries/System.Net.Mail/tests/Functional/ || break; done
Environment:
- dotnet/runtime master on osx arm64
Reproduction Steps
.
Expected behavior
.
Actual behavior
.
Regression?
No response
Known Workarounds
No response
Configuration
No response
Other information
No response
Known Issue Error Message
Fill the error message using step by step known issues guidance.
{
"ErrorMessage": "",
"ErrorPattern": "Console log: 'System\.Net\.Mail\.Functional\.Tests' from job(.|\n)*mono_dump_native_crash_info",
"BuildRetry": false,
"ExcludeConsoleLog": false
}Report
Summary
| 24-Hour Hit Count | 7-Day Hit Count | 1-Month Count |
|---|---|---|
| 0 | 0 | 0 |
Activity
vargaz commentedon Jun 8, 2023
@BrzVlad
BrzVlad commentedon Jun 8, 2023
BrzVlad commentedon Jun 8, 2023
Looks like
pthread_exitcrashes ? Doesn't make much sensevargaz commentedon Jun 8, 2023
I get random crashes inside the interpreter locally:
BrzVlad commentedon Jun 9, 2023
The way this issue reproduces is that when doing a call
ipis fetched as null fromcmethod->code. This pointed to the idea that there is incorrect memory synchronisation betweenimethod->transformedandimethod->code.The following fix solves the issue:
Adding a read barrier in the fast path for each call might be quite expensive. It might be worthwhile benchmarking though.
Another possible fix would be to rely on data dependency to ensure data is accessed correctly. We could extract imethod compilation data in a separate structure that is published in the imethod when compilation is done and then, during method execution, access this through it:
imethod->transform_data->code,imethod->transform_data->alloca_sizeetc. This would add a few ptr derefs in the fast path so it probably would have some penalty cost, I assume lower than a read barrier for every call.Another fix that would add no overhead for calls, would be to reuse the existing idea used for tiering and patch with new imethods. We would have a separate
InterpMethodfor compiled method and when we compile it, we patch the data items with the new pointer, relying once again on data dependency to ensure the compilation data is accessed correctly.vargaz commentedon Jun 9, 2023
Wouldn't getting rid of
transformedand checkingcodeinstead solve the issue ?BrzVlad commentedon Jun 9, 2023
The same logical problem still remains, that if the method is transformed we expect certain fields to have been properly initialized. We could check for
codeand maybe this particular issue is fixed, but then immediately during executionimethod->alloca_size,imethod->data_itemsetc. could also theoretically be invalid.