add thread safety testing and add synchronized for thread safety #42
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: SamYuan1990 <yy19902439@126.com>
SamYuan1990
requested review from
xiaohui249 and
davidkhala
and removed request for
xiaohui249
xiaohui249
reviewed
Jan 12, 2021
| IvParameterSpec ivParameterSpec = null; | ||
| if (iv != null) { | ||
| ivParameterSpec = new IvParameterSpec(iv); | ||
| synchronized (this) { |
There was a problem hiding this comment.
看了这段同步代码，以及SM4Util的公开构造方法。我的想法：1、改成synchronized，目的应该是避免一个SM4Util单实例使用出现线程安全问题，但是影响了性能，而且并没有从实质上解决线程安全问题，线程安全问题是由Cipher引起的，而Cipher对象是公用的；2、SM4Util的构造方法是public的，所以从这个层面来讲，上一步的synchronized意义不大，而且实例化SM4Util时，Cipher.getInstance仍然要执行一遍。
There was a problem hiding this comment.
哦？但是奇怪的是我们的并发测试貌似通过了。@xiaohui249, 建议提交个pr我们看下区别。
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: SamYuan1990 yy19902439@126.com