This PR adds support for Array of JavaScript References on the JavaScriptInvoke node. Existing code in ASW does not use this code path but it is being reserved for the future.
As part of this change the following bug fixes were made:
Close Reference did not properly free JavaScript References when a reference was closed.
The reference was made invalid however the JS maps containing the reference were not freed. This resulted in a memory leak for JS values that were expected to be freed.
readJavaScriptRefNum incorrectly returned undefined for invalid JavaScript References instead of throwing an Error.
This was a bug because undefined is also a valid value to store in a JavaScript Reference. By returning undefined for an invalid reference we made it impossible to distinguish between an invalid reference and a references holding the value undefined.
Dynamic JavaScript references and Static JavaScript references used the same map and the same map reverse lookup code. This would cause errors in the following scenario:
A static reference is made to a JS value in memory
A static reference is created that retrieves the existing cookie value in the reverse lookup map that was overwritten by the dynamic reference. The static reference now has a dynamic reference cookie.
The dynamic reference closes its reference which also destroys the static reference using the same cookie.
This bug cannot currently arise in practice because static reference creation is not interleaved with dynamic reference creation as the editor emits all static reference creation first. However, it is placing the system in an invalid state.
Implementation
Separated the backing maps into StaticRefnumManager and DynamicRefnumManager. This lets them each place stronger asserts based on the reference type. It also encapsulates behaviors such as reverse lookups in each map code.
This does not preclude sharing the maps in the future if needed, ie dynamic maps could fallback to shared maps for lookups if needed.
Fixed eggshell readJavaScriptRefNum to throw for invalid refnums and added the complementing isJavaScriptRefNumValid api to allow checking before reading. Also added clearJavaScriptRefNum eggshell api for completeness allowing the eggShell api to create, read, write, and destroy a JS reference. Added tests for all of the eggShell JavaScript Reference apis since none existed.
Added a case to parameterValueVisitor and returnValueVisitor for the JavaScriptInvoke node to support Arrays of JavaScriptReferences. Added DataValue and ReturnDataValue tests for both JavaScript References and Array of JavaScript References since those tests did not exist.
Added a test to verify that calls to Close Reference do get removed from the DynamicRefnumManager
Description
This PR adds support for Array of JavaScript References on the JavaScriptInvoke node. Existing code in ASW does not use this code path but it is being reserved for the future.
As part of this change the following bug fixes were made:
Close Reference did not properly free JavaScript References when a reference was closed.
The reference was made invalid however the JS maps containing the reference were not freed. This resulted in a memory leak for JS values that were expected to be freed.
readJavaScriptRefNum incorrectly returned
undefinedfor invalid JavaScript References instead of throwing an Error.This was a bug because
undefinedis also a valid value to store in a JavaScript Reference. By returningundefinedfor an invalid reference we made it impossible to distinguish between an invalid reference and a references holding the valueundefined.Dynamic JavaScript references and Static JavaScript references used the same map and the same map reverse lookup code. This would cause errors in the following scenario:
This bug cannot currently arise in practice because static reference creation is not interleaved with dynamic reference creation as the editor emits all static reference creation first. However, it is placing the system in an invalid state.
Implementation
Separated the backing maps into StaticRefnumManager and DynamicRefnumManager. This lets them each place stronger asserts based on the reference type. It also encapsulates behaviors such as reverse lookups in each map code.
This does not preclude sharing the maps in the future if needed, ie dynamic maps could fallback to shared maps for lookups if needed.
Fixed eggshell readJavaScriptRefNum to throw for invalid refnums and added the complementing isJavaScriptRefNumValid api to allow checking before reading. Also added clearJavaScriptRefNum eggshell api for completeness allowing the eggShell api to create, read, write, and destroy a JS reference. Added tests for all of the eggShell JavaScript Reference apis since none existed.
Added a case to parameterValueVisitor and returnValueVisitor for the JavaScriptInvoke node to support Arrays of JavaScriptReferences. Added DataValue and ReturnDataValue tests for both JavaScript References and Array of JavaScript References since those tests did not exist.