On Fri, 22 Oct 2021 18:45:31 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> The current code that changes cipher suites disposes the new suite instead >> of the old one, which usually silently fails. This patch fixes the code to >> dispose the old instance instead. >> >> DTLS appears to be unaffected: DTLSOutputRecord keeps 2 ciphers and >> correctly [disposes the old >> one](https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/src/java.base/share/classes/sun/security/ssl/DTLSOutputRecord.java#L106), >> and DTLSInputRecord [doesn't dispose >> anything](https://github.com/openjdk/jdk/blob/4b9303b77b43d890ebacbec38b4ac5db7e171886/src/java.base/share/classes/sun/security/ssl/DTLSInputRecord.java#L57) > > Did you want to cover the update for line 222 at OutputRecord.java as well? After reviewing the scope of changes to fix writeCipher disposal I decided to remove it entirely. It would probably be a nice follow-up enhancement, but I'm not confident I'd implement it correctly on the first try, so I'd rather not introduce it in a bugfix PR. @XueleiFan is that acceptable to you? On a side note, I had another look at [DTLSOutputRecord](https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/src/java.base/share/classes/sun/security/ssl/DTLSOutputRecord.java#L106). It looks like we keep a reference to `prevWriteCipher` only to call `dispose()` during the next cipher change. I admit I don't know how often DTLS changes ciphers, but I think GC would have collected that cipher earlier if we didn't keep it. ------------- PR: https://git.openjdk.java.net/jdk/pull/6084
