Opened 11 years ago

Last modified 10 months ago

#1879 reopened defect

Reliably serialize NaN values

Reported by: historic_bruno Owned by: wraitii
Priority: Must Have Milestone: Backlog
Component: Core engine Keywords:
Cc: Patch: Phab:D3205

Description (last modified by temple)

In #1828, it was discovered the cause of serialization test failure was Spidermonkey using different values internally for NaN. According to ECMAScript-262, there is only a single NaN value which can have different internal representation depending on the implementation. Because this might vary on different systems and even the same system in JIT vs. non-JIT paths, we should probably canonize it when serializing so it doesn't cause an OOS or serialization test failure.

Related problem: the debug serializer doesn't handle NaN or Infinity, so a simple text diff won't reveal these errors. For floating-point values in the engine, we can fix that by completing the canonfloat() function. For script vals, it's a bit more difficult because we use JS_Stringify, which per the JSON spec doesn't allow NaN or Infinity (replacing them with null).

Change History (12)

comment:1 by ben, 11 years ago

In 13322:

Fixes serialization test failure caused by CmpFoundation's buildMultiplier property sometimes being NaN (which can assume different internal values in Spidermonkey). For now, NaN cannot be reliably serialized. Fixes #1828, refs #1879

comment:2 by Yves, 10 years ago

Owner: set to Yves
Status: newassigned

I'm taking this ticket because I currently work with SpiderMonkey and Serialization which is both related and I will probably run into that problem too (or figure out it's solved now).

comment:3 by Yves, 8 years ago

Owner: Yves removed
Status: assignednew

comment:4 by temple, 6 years ago

In 21231:

Avoid NaN in Foundation and Repairable

Differential Revision: https://code.wildfiregames.com/D1296
Reviewed by: elexis
Comments by: bb, mimo
Fixes: #5030
Refs: #1879

comment:5 by temple, 6 years ago

Description: modified (diff)
Milestone: BacklogAlpha 24
Priority: Nice to HaveShould Have

Some discussion at Phab:D1296.

comment:6 by Stan, 3 years ago

Patch: Phab:D3205

comment:7 by wraitii, 3 years ago

Milestone: Alpha 24Alpha 25

Pushing. There are no known cases.

comment:8 by wraitii, 3 years ago

Owner: set to wraitii
Resolution: fixed
Status: newclosed

In 25151:

Refuse to serialize NaN values.

NaN values could not be serialised safely because of the multiple possible NaN numbers.
Since NaN values are usually the result of bugs or dangerous code, it seems simpler to refuse to serialise them.

(D3205 was a safe-serialization alternative, should the need arise).

Fixes #1879

Differential Revision: https://code.wildfiregames.com/D3729

comment:9 by Freagarach, 3 years ago

Milestone: Alpha 25Alpha 26
Priority: Should HaveMust Have
Resolution: fixed
Status: closedreopened

comment:10 by Stan, 2 years ago

Milestone: Alpha 26Alpha 27

Unlikely to get done for A26

comment:11 by Freagarach, 14 months ago

Milestone: Alpha 27Backlog

Pushing back.

comment:12 by wraitii, 10 months ago

I think the solution would be to not crash in case of serialisation error, not trying to fix the underlying problem.

Note: See TracTickets for help on using tickets.