openbmc / bmcweb

A do everything Redfish, KVM, GUI, and DBus webserver for OpenBMC
Apache License 2.0
156 stars 131 forks source link

HTTP core should use AsyncResp everywhere #179

Closed edtanous closed 3 years ago

edtanous commented 3 years ago

bmcweb::AsyncResp is a class that was created to handle http responses in an RAII fashion, sending to the user when the final reference has been deleted. This improvement would be to propagate AsyncResp to the entire core http layer, such that no handler is accepting requests by const Request&. Some key points of design:

  1. The HTTP connection class would need to move from an http Response value, to a shared_ptr. This would mean that if the connection closes during the handling of a request, the connection could be destroyed immediately, with a null handler being placed in res.completionHandler, as the response object could live much longer than the connection.
  2. All URI handlers would need to be migrated to use AsyncResp. Most of the new code already does, but there's some old code in some places that still uses res.end(). Porting should be trivial, as anytime we've moved res.end() based code to AsyncResp, we've never encountered a bug.
  3. Redfish uses a different AsyncResp class than the rest of bmcweb. They are very similar, part of this work would involve removing the redfish AsyncResp entirely.

Once this was completed, this would also allow recursive calls of app.handle() from a handler, as the reference counts wouldn't need to be reset and re-inited every time.. This would allow for doing operations like filter, expand, and only query params, by simply re-calling handle, and merging the various AsyncResp response bits where needed. (Note, this probably needs further designed).

edtanous commented 3 years ago

This is resolved on master.