Skip to content
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

src/client.coffee: side-effect of using shared syncmsg in syncRespHandler #85

Open
tushar-rishav opened this issue Jun 19, 2019 · 0 comments

Comments

@tushar-rishav
Copy link

Hi @stianeikeland ,

In a very rare scenario when there are multiple async requests fired almost in parallel via single client, all such requests share the same syncmsg attribute for the Client object. This triggers a situation when state/response of requests interfere with each other. IIUC, the scenario when this happens is after immediately event loop is unblocked at deasync.loopWhile and the next request in pipeline (somehow) gets scheduled, essentially modifying the shared state syncmsg.
To be honest, the above just seems absurd and assuming there's no notable async step once deasync.loopWhile is unblocked, theoretically the node should have continued the execution until returning in the below lines in the same tick:

delete options.syncdone
return @syncmsg

But, somehow, that's not happening. As a workaround, I tried to bind the state with req object which is new for each request and that worked for me.
The patch is as below:

diff --git a/src/client.coffee b/src/client.coffee
index 1ed7847..e11de50 100644
--- a/src/client.coffee
+++ b/src/client.coffee
@@ -85,15 +85,16 @@ class Client
       # Deliver response
       @_handleResponse err, resp, body, callback
 
-    syncRespHandler = (err, body, headers) =>
+    callback = syncRespHandler if options.synchronous is true
+    req = @_doRequest options, reqRespHandler
+
+    syncRespHandler = (err, body, headers) ->
       options.syncdone = true
-      @syncmsg =
+      req.syncmsg =
         err: err
         body: body
         headers: headers
-    callback = syncRespHandler if options.synchronous is true
 
-    req = @_doRequest options, reqRespHandler
     token.setRequest req
 
     if options.synchronous is true and options.syncdone is undefined

Look forward to hearing your thoughts.
Thanks

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

No branches or pull requests

1 participant