-
Notifications
You must be signed in to change notification settings - Fork 795
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
System.NullReferenceException in new task
CE in F# 6
#12359
Comments
@sasmithjr Thanks for opening this issue, I can repro it. @dsyme in a normal project this repro causes an AccessViolationException which usually points to invalid IL, the dynamic case seems to suffer from the same underlying issue though just manifesting as an NRE instead. An interplay of resumable code and calls to a function with InlineIfLambda arguments before the first yield seems to be the trigger (moving it after a resumption point indeed avoids the issue). |
Thank you for the bug report, we will prioritise a fix for this.
It could also be the presence of an inlined loop implied by |
we see similar things. Sometimes NRE and sometime |
all issues that I saw today were with Array.map somewhere in that code |
The fix is in. I wrote up some workarounds that can be used systematically until dev 17.1 and its associated update to .NET SDK and FCS are out |
Happy to hear this is fixed! Anyone know when it'll be out - days, weeks, months? I have some (non-critical) work that depends on this (through Giraffe). |
I'd imagine it's in preview within weeks, though @KevinRansom and @brettfo may know for sure |
@KevinRansom @brettfo do you know if this was included in the 6.0.101 SDK that came out today or was that only for the CVE? |
This was a fix for CVE, tasks fix is in 6.0.200:
Not sure where a non-preview version will be released exactly. |
@KevinRansom @dsyme @KathleenDollard Any idea of when this is going to go in? We want to upgrade SAFE Stack to .NET 6 but this is kind of a blocker - it doesn't really make sense to release a .NET 6 version of SAFE Stack which doesn't use the flagship feature of F#6, but at the moment in all good conscience we can't release the latest template with a bug like this. I appreciate that there are release windows for this sort of thing and it all has to be scheduled in, but it's been over two months since @dsyme got a fix out for this and we don't even know when this is going out. If there is a public release plan for this, my apologies - just send me the link to that. Otherwise, could we at least have some idea as to when this is going to be released so that we can start to put in place some kind of release plan for SAFE? This is harder than it sounds, since we would need to align all the downstream dependencies e.g. Saturn, Giraffe etc. to ensure that it's all in lockstep and the dependency tree is tested out. Thanks! |
@isaacabraham SDK integrations have been continuing at their normal pace, so I think it's in the 6.0.2xx series and coming through the pipe https://github.com/dotnet/installer/blob/main/README.md#installers-and-binaries. I don't think there's any particular delay - perhaps the usual few weeks over US holiday season. @KevinRansom can confirm. I believe you can upgrade to .NET 6 without using F# 6 tasks - just continue to use FSharp.Core 5.0.x, plus whichever task implementation you're curently using. If yo ulike set `/langversion:5.0" though I don't think you need to - F# 6 can be used just fine with FSharp.Core 5.0.x, just some features like built-in tasks will not be available. You are right to wait until this fix is available in a public release before updating the stack to both F# 6 + FSharp.Core 6.0.x. |
Understood - wasn't really complaining that it's taking long (although IMHO this should have been released much sooner). It's more just the visibility and clarity that would help us.
Yes - sure it is. But we really don't want to be doing this with our customers, it's just more work - better to just wait until the fix is out. Especially with the SAFE - there are so many moving parts, a hybrid F# 5/6 solution would really be undesirable. |
I sympathise. As an aside, upgrading a stack of tech is really painful - in particular I wish there was a way to coordinate a single PR over multiple repositories (Giraffe, Saturn, SAFE) , with all of them doing CI runs, and publication of packages propagating within the overall PR. The only real way of making this efficient is to merge more and more repositories. |
Release expected in a week dotnet/runtime#64529 (comment) |
@dsyme I'm not sure this was fixed. Using .NET SDK 6.0.102, Using macOS arm64, this is what I get when I try to see if it still reproduces in FSI:
Same issue in a brand-new console app:
|
@cartermp I think the fix is targeting .200 sdk. 102 was fixes only afaik. |
Aha. Any clue as to when that one will drop? We're pretty angsty about getting this fix in so we can update the F# web ecosystem |
Supposed to be 2nd Tuesday of the February, according to dotnet/runtime#64529 (comment) Not sure whether plans changed. @brettfo do you happen to know if it's gonna be released anytime soon? |
FYI, .NET SDK 6.0.102 has been released yesterday, and it's included in VS 2022 17.0.6 After looking and trying this update, the fix I think the fix isn't included in this SDK 6.0.102 release... |
Ah I see, for some reason I thought .200 was set to release yesterday. |
@vzarytovskii Therefore, 6.0.1xx will always be available with VS 2022 17.0.x updates, whereas 6.0.2xx will always be available with VS 2022 17.1.x updates. This weird theme has been there since .NET Core 3.x and 5.0 releases. NOTE that those SDK has the same runtime version, but the SDK is for different release of VS 2019 16.11.x and 16.9.x So I hope this helps you, @dsyme , @cartermp and others as well 🙂 🙏 |
Additional note: (based on my previous comment) Due to the fact that VS 2022 17.1 is still in Preview 2 and there will be another next preview of 17.1.x, I think the RTM of 6.0.200 SDK will have to wait for VS 2022 17.1.0 release. |
6.0.200 is available now, and it seems to include the fix for this issue. |
Looking at the download page of .NET 6.0 SDK, it seems we haven't got the fix released As we have known, the 6.0.2 runtime doesn't include this fix. It is still the same as the runtime used in 6.0.101 then. So looks like we actually have to wait for updated SDK (maybe 6.0.201 for VS 2022 17.1.x and 6.0.103 for VS 2022 17.0.x) with the updated runtime of 6.0.3. To be honest, I'm fine with 17.1 but at the same time this 6.0.200 SDK has the same problematic 6.0.2 as described in the initial issue. FYI, I personally wait for 6.0.3 to be actually released. |
@eriawan - This fix is in the FSharp.Core shipped with 6.0.200, which is independent from the 6.0.2 runtime. Here's the code from the report in a 6.0.200 dotnet fsi that I just installed:
|
Yeah, most of F# fixes land in SDK (both compiler and FSharp.Core). There are some exceptions where runtime contains the change to account for F#'s codegen (for example, most recent one is some improvements for .tail calls). |
In some cases where a
let
precedes alet!
in the new task CE, an NRE is thrown. I'm not sure of the specifics for why it happens sometimes and not others, but I've got an example than can quickly be run through FSI.dotnet --version
: 6.0.100OS: macOS 11.6.1
Repro steps
After using
dotnet fsi
, copy/paste the following code block into FSI to see the issue:If you swap the
let w...
andlet! x...
lines, it works:Or a simple replacement of
Array.map
andArray.head
works:Adding a second
let
also works:Actual behavior
System.NullReferenceException: Object reference not set to an instance of an object. at FSI_0003.z0@13.MoveNext()
Expected behavior
I'd expect the
z0()
call to eventually evaluate to8
instead of throwing an exception.Known workarounds
The problem occurs when
task { ... }
, andlet v = ...
where thev
becomes a state variable of the task state machine, because it is used after an asynchronous pointwhile
ortry
orfor
or a construct such asArray.map
that is inlined to produce one of theseIn short, the most common cause appears to be the use of the inlined
Array.map
insidetask { ... }
. For this case, you can either rewrite your code to avoid the use ofArray.map
, e.g. usingArray.mapi
, or define and use a non-inlinedArray.map
as follows:Alternatively using
Array.mapi
:Other workarounds are shown above
The text was updated successfully, but these errors were encountered: