ModelInference / perfume-frontend

Web-based frontend for Perfume tool
Other
1 stars 0 forks source link

Race condition when multiple backend requests are submitted #100

Closed bestchai closed 4 years ago

bestchai commented 4 years ago

It is possible to submit multiple backend requests before any of the corresponding responses are received by cancelling the current request and submitting another request. This causes a race condition when the responses are received and each one is processed in turn.

To reproduce, click Infer model for the djQ example, click the X on the "Inferring model..." dialog, then switch to the browser caching example and click Infer model again. Both requests will be submitted to the backend, and both responses will be processed when they are received. If the backend response for djQ is received second, the model and invariants will be relevant to the djQ log, but the input pane will show the browser caching log and args.

We should make the frontend only accept backend responses for the newest frontend request. This can be accomplished by keeping a monotonically increasing requestID that is included with each request sent to the backend. The frontend should then only process a backend response if responseID matches the current requestID. Increase requestID whenever a request is "finished," that is when the corresponding response has been received (and is about to be processed) or when the request has been canceled (by clicking the X on the "Inferring model..." dialog).

[Issue created by ohmann: 2016-04-05] [Last updated on bitbucket: 2016-04-27]

[Comment created by ohmann: 2016-04-27] Merge. Resolved issue 110

→ <>

[Comment created by wborkofsky: 2016-04-21] Solution in revision 036f5d9, please review.

[Comment created by ohmann: 2016-04-21] Reminders from the meeting:

  1. You're free to pretty-print / nicely format the ajax request so it's not so ugly and difficult to understand.
  2. The response will contain a field responseID; only increment requestID if they match. As was always the plan, also increment if the user cancel's the request.

I renamed the field within parameters requestID rather than request so that it's obvious the field contains an ID and not an actual full request, and I updated the backend so that it accepts your requestID and returns the responseID (upon both success and failure). Make sure you pull my changes, and then go ahead and finish the issue!

[Comment created by ohmann: 2016-04-19] Sorry for my delay in reviewing your code. I'll need to delay a bit longer, but tomorrow afternoon, I'll be making the backend read the requestID and include a responseID, which will make it possible to test your full solution to this issue. Feel free to already begin work on processing the responseID (assuming you'll receive it, which I know you don't yet), knowing you'll be able to test it in around 24 hours.

[Comment created by wborkofsky: 2016-04-16] I tried adding the first part, sending along a requestID within each request. Please review revision 1f3e237.

[Comment created by ohmann: 2016-04-12] Fixing this bug requires not only the changes in the frontend to support sending and processing requestIDs/responseIDs but also some changes in the backend. I'll take care of the backend changes when the frontend at least does the first part described above, sending along a requestID within each request.