vanzin commented on issue #26609: [SPARK-29971] Fix multiple possible buffer leaks in `TransportFrameDecoder/TransportCipher` URL: https://github.com/apache/spark/pull/26609#issuecomment-557201597 > I am not sure it is safe to assume it always consume all data as an InternalError I'm not sure I follow you. If there's an internal error the whole channel is pretty much gone, nothing gets transferred anymore. In the caller, you just do a `try...finally` and release the buffer in the finally. In the absence of errors, the contract is that all data in the buffer fed to the `feedData` method is read by `TransportCipher`. Again, I understand you're thinking of this as generic code anyone can use, but it's not. This is internal Spark code that serves a specific purpose, behaves in a very particular way (it actually only exists to work around a bug in the commons-crypto library) and is not used anywhere else. In other words, the buffer copying code you're adding is basically dead code. If it's ever triggered it's actually a bug in the existing code, so instead of adding that code you should be adding a check and throwing an exception, as I suggested.
---------------------------------------------------------------- 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] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
