-
Notifications
You must be signed in to change notification settings - Fork 991
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
Broken IDispatch.GetTypeInfo interop tests #3305
Comments
I can reproduce these exceptions inside the VS debugger, but I don't know how to diagnose this without further assistance. Things I checked:
What else can cause this exception? I have a dump taken from debugger here. If I can assist diagnosis in other ways feel free to make suggestions. /cc @AaronRobinsonMSFT @JeremyKuhne for COM interop issues |
I remembered that I still don't see how letting the GC cleanup an RCW refcount can cause this particular exception though, is this something that could be a bug in the coreclr RCW finalizer and needs further investigation, or should we just ignore this and be working around by manually managing |
I remember in #2603 (#2603 (comment)) we had some problems. Maybe they are realted? |
Right, this is probably related to the linked comment. It'll probably mean we have to do manual RCW reference management to avoid the problem of automatic release, I'll do the corresponding PR. I still want the opinion of @AaronRobinsonMSFT on whether this may be a race condition in the runtime that needs further inspection, or if we should ignore it and work around it. |
I also made this a while ago: https://github.com/hughbe/Interop-Repro Run StressTest.ps1 from the root of the repo |
Having read through the whole linked thread it looks like this issue was exactly what happened before. The previous workaround was probably relying on xunit.stafact behavior, now that its updated the old workaround might no longer work. The correct workaround is to never cause the problematic situation, if you don't want to wind up individual processes for tests the easiest solution is manual RCW reference management. Considering that this scenario already has been examined there's probably no desire to fix anything in classic COM interop. I'll be preparing the PR to do manual RCW reference management then, that should fix the issue reliably for the future. |
Sorry about missing this. @hughbe Yes, this issue is the same as #2603 (comment). This is a general problem with the XUnit framework and the STA for concurrent tests. @weltkante Your suggestion about manual |
Thank you all 🚀 |
Sorry, missed one ITypeInfo 😞 Want me to make another PR for that line or leave that for the nuget package update? Around 1 out of 10 runs fails locally so I missed it. Its definitely better than all tests failing constantly. |
Let's fix it :) |
.NET Core Version:
Master with PR #3276 applied
Have you experienced this same bug with .NET Framework?:
unknown
Problem description:
The tests were flaky before, sometimes throwing
ComException(E_FAIL)
. The update of xunit.stafact now is properly hosting everything in an WinForms STA thread and they suddenly seem to fail "reliably" with this error here.Expected behavior:
Tests pass or there is a reasonable explanation for the error
Minimal repro:
Run tests with PR #3276 applied
The text was updated successfully, but these errors were encountered: