Remove usage of AtomicIntegerFieldUpdater in ReferenceCountedOpenSslE…#9653
Remove usage of AtomicIntegerFieldUpdater in ReferenceCountedOpenSslE…#9653normanmaurer merged 4 commits into4.1from
Conversation
…ngine Motivation: There is not need to use a CAS as everything is synchronized anyway. We can simplify the code a bit by not using it. Modifications: - Just remove the CAS operation - Change from int to boolean Result: Code cleanup
|
@njhill you brought this up at some point... |
|
How about this access from wrapped delegated tasks, not sure that it's synchronized? |
Guess we keep the field volatile then? |
handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
Show resolved
Hide resolved
|
@normanmaurer there's at least one comment referencing DESTROYED_UPDATER which should be updated. Thumbs up to the rest of the diff, it should behave the same as before, that said it looks like there might be a race condition when executing the delegated task where T1 checks destroyed boolean and continues then T2 invokes shutdown concurrently (delegated task running after the ssl engine has been destroyed).. I'm afk right now so I cannot run any local tests. |
|
@johnou thats as every access to the engine is guarded and so not a problem when running the delegating task |
johnou
left a comment
There was a problem hiding this comment.
There's some bitrot with a comment referencing DESTROYED_UPDATER but the rest lgtm.
|
@johnou yep will fix |
#9653) Motivation: There is not need to use a CAS as everything is synchronized anyway. We can simplify the code a bit by not using it. Modifications: - Just remove the CAS operation - Change from int to boolean Result: Code cleanup
…ngine
Motivation:
There is not need to use a CAS as everything is synchronized anyway. We can simplify the code a bit by not using it.
Modifications:
Result:
Code cleanup