Skip to content

Conversation

@jamaljsr
Copy link
Member

@jamaljsr jamaljsr commented Feb 24, 2023

Currently, the JS callbacks for onlocalprivcreate, onremotekeyreceive, and onauthdata must be on the JS global scope for the WASM to call them. This adds a potential for race conditions when connecting to multiple nodes simultaneously because each connection will share the same callback functions that are defined on the global scope. Depending on the order in which the connections are established and callbacks are called, the local/remote keys returned may be associated with the wrong connection.

This PR allows the app to specify callbacks nested under a JS object, ex: '--onauthdata=myapp.onAuthData'. The app would have needed to create the nested object like this, for example:

window['myapp'] = {
  onAuthData: function(data) { ... }
}

The app can also leverage the support for namespaces by placing the callbacks on a JS object using the same name as the namespace, ex '--onauthdata=' + namespace + '.onAuthData'. This would eliminate the chances of a race condition because each connection would have its own callback function defined under a namespace scoped JS object.

I implemented this in a way that maintains backward compatibility, so apps can continue to use '--onauthdata=onAuthData' with the callback defined on the global scope if they choose to. It just now supports the option of using non-global scoped callbacks to handle the scenarios where multiple connections are established concurrently.

@jamaljsr jamaljsr requested a review from ellemouton February 24, 2023 16:27
Copy link

@itsrachelfish itsrachelfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK using the example-server

Copy link
Member

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice 🔥

@jamaljsr jamaljsr merged commit 5903579 into master Mar 1, 2023
@jamaljsr jamaljsr deleted the avoid-global-scope branch March 1, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants