stianeikeland / node-etcd

:satellite: Etcd client for nodejs
BSD 3-Clause "New" or "Revised" License
259 stars 85 forks source link

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

Open tushar-rishav opened 5 years ago

tushar-rishav commented 5 years ago

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