sudheerv commented on pull request #7285: URL: https://github.com/apache/trafficserver/pull/7285#issuecomment-716668469
> What are the cases where the server_netvc is doing a read from a different thread? The server_netvc and the client_netvc and the HttpSM are all associated with the same ET_NET thread for the duration of the HttpSM. So all the net I/O should be performed in the same ET_NET thread. In that case the buffer frees in Http1ServerSession::do_io_close are serialized with the READ/WRITE event processing. > This seems to happen when we use a plugin which creates response Transform. We've a couple of custom plugins that perform Server Side Rendering (sort of like combo_handler plugin) by parsing a HTML origin response and fire off sideways calls to fetch the assets listed in the HTML response and then eventually stitch those responses together following a template (HTML in some cases, dust in some cases). The sideways requests are performed using TS Fetch API. I tried to set the thread affinity in the transform continuation and the sideways Fetch calls, yet couldn't seem to force all the associated requests to the same thread. > In my reading of the current master, we may need to add do_io_read(nullptr, 0, nullptr) and do_io_write(nullptr, 0, nullptr) in the Http1ServerSession::do_io_close to ensure that a later net/io event does not get sent back to the now deleted continuation. But if the server_vc is still set, the closed flag should already be set which should block any future net/IO on that netvc. The problem is do_io_close() is called from SM/Tunnel continuations which presumably guard using SM/Tunnel mutex. But, somehow that doesn't seem to block the race condition which makes me think that when Transforms are at play, the network read VIO isn't really being guarded by the SM/Tunnel Mutex anymore. Either way, it feels that once do_io_close() is called and it resets the continuation (or even a do_io_read(null, 0, null) for that matter) the network i/o is unblocked and potentially try to use the now freed MIOBuffer for the read. What we really need is a dedicated mutex for net read and use that mutex to lock the SM/Tunnel as well. But. I did not want to affect the performance for the normal cases as well as also introduce complexity by having to handle such lock failures in the SM/Tunnel continuations. So, as a compromise, I basically deferred/delayed the buffer release until the server session is detached from the SM and is put on the idle pool at which point adding an add itional mutex should not impact request processing or latency. We can discuss more on slack/zoom or during the summit. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
