-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.Text.Json.JsonSerializer.Deserialize crashes when parsing large string using Blazor #41604
Comments
@mkArtakMSFT could you label/assign this appropriately. |
Is the property starting with |
No, not at all. Every time it reports something that is not there. |
@benaadams : Row # 63547 (first row =1) is an "ID" property and row 63547 (first row =0) is a "DateTime" property. |
It is just an array of instances all with the same properties as shown above. |
@erikthysell are you able to share a small repro app (sanitized if necessary) that highlights the issue? This would include all types being (de)serialized, any custom converters, and the JSON payload being passed to the serializer. Also, can you try using .NET 5 preview 8 and see if you still face the issue? |
@layomia : Sure.. I hope that is all that is needed... |
@layomia : I can confirm that it happens using .NET 5.0 preview 8 also. But it seems to be working faster and reaches higher row/line numbers before it throws an error.
|
@erikthysell thanks a lot. I'll take a look. |
Just grabbed this data file and checked it out. The offending double array looks like this:
Looks fine to me. This isn't the first occurrence of a double without a decimal point in the file. @layomia were you able to repro? @erikthysell did this only repro in blazor? When it repros is it always on the same position in the JSON? |
@ericstj : I have only tried with blazor wasm (client side) core 3.1 and .net 5 preview 8, chrome 85ish and edge 44. Always different position, sometimes the Lev array othertimes other properties. |
I was able to repro this with in a blazor wasm app as described by @erikthysell and also in a test CI run (#42004) where the scenario fails for blazor wasm and Mono interpreter builds. The issue doesn't repro in a plain console app (CoreCLR JIT). Trying to root-cause this now. The JSON payload, while very large, is fine and shouldn't cause exceptions to be thrown here. Might be an error with the IndexOfQuoteOrAnyControlOrBackSlash method, similar to the issue in #41582 (comment). fwiw, the deserialization appeared to work as expected when I refactored the repro code slightly to use the |
@layomia in order to help determine interpreter issue vs. FWIW for performance, this method was one that was considered for intrinsifying in the interpreter or re-write in C# for mono but that never happened. See #41097, #40705 and #39733. |
@layomia thanks for the tip about the async version! I can just confirm that it works. |
@steveharter thanks! I ran the tests with |
Based on additional testing by @layomia by using a simple implementation of IndexOfQuoteOrAnyControlOrBackSlash, this is now considered an issue in the mono\wasm area and not STJ. Async may be working because it allocates a lot less memory than the non-async version. The async mode starts with a 16K buffer and will re-use that for all JSON from the Stream until there is a property value that is too large to fit in the remaining space left in the buffer, then it will double the buffer, etc. So a 110MB payload will not allocate anywhere near 110MB in async mode unless the 110MB is a single property value... |
Tagging subscribers to this area: @CoffeeFlux |
@lewing @marek-safar - possibly RC candidate |
@SamMonoRT agreed |
This doesn't look related to Json or the runtime, therefore not a release blocker. It seems to be an issue on the JS interop layer in a third party library. Submitted bug Tewr/BlazorFileReader#161. |
I believe this was fixed by #42486, which should be backported soon. Can you validate that fix if you have the repro handy? |
@CoffeeFlux it was not fixed |
Gotcha. If so, we probably should close the issue or move it off 5.0. |
This was an issue in the library not the runtime and discussion has moved to Tewr/BlazorFileReader#161. Closing. |
This issue has been moved from a ticket on Developer Community.
[severity:Other] [regression] [worked-in:somewhere before 16.7.1]
Using Blazor/Wasm (3.2.0) and trying to parse a large json-string (135123600 chars) loaded from a file on my local windows machine, System.Text.Json.JsonSerializer.Deserialize crashes with the following internal exception (or similar, varies somewhat from run to run):
Line number of the file and position varies where it fails /(and therefore what is expected (number, string or property name).
The string contains a list of objects like:
and so on. (all in all 604621 instances).
(so far tested on Chrome 84.0.4147.135 and Microsoft Edge 44.18362.449.0)
Original Comments
Feedback Bot on 8/27/2020, 01:09 AM:
We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.
Original Solutions
(no solutions)
The text was updated successfully, but these errors were encountered: