fix hession maven scope and addConnectionEventProcessor concurrency problems #321
Conversation
|
If user do not need hessian，then he/she can not remove hessian from maven. I think it maybe unreasonable. |
I don't think so, if user need another serialization protocol, it is ok, but not necessary to delete it. |
chuailiwu
changed the title
OK, it's ok to keep hessian in dependencies. |
| public class ConnectionEventListenerTest { | ||
|
|
||
| @Test | ||
| public void addConnectionEventProcessorConcurrentTest() throws InterruptedException { |
There was a problem hiding this comment.
This unit test can not reproduce the bug.
new Thread(thread).start(): Thread is started in order. So,rpcClient.addConnectionEventProcessor(ConnectionEventType.CONNECT, (remoteAddress, connection) -> {});may called in order. We can add a CountDownLatch beforeaddConnectionEventProcessormethod, soaddConnectionEventProcessorcan be concurrency.fail()method is called in sub thread . We can not see it in main thread.- ArrayList maybe thread safe for add method.
There was a problem hiding this comment.
look at issue "Steps to reproduce"
#309
There was a problem hiding this comment.
When it change 'CopyOnWriteArrayList' to 'ArrayList', this unit test cannot throw any Exception.So I say this unit test can not reproduce the bug. Please pay attention to it. @chuailiwu @EvenLjj
There was a problem hiding this comment.
This unit test indeed can not reproduce the bug, even I add a CyclicBarrier to control all threads run in same time.
But the fix is right, there is a concurrency issue here.
If you get the list size, you will find it smaller than it shoud be when using ArrayList.
Improve UT:#324
There was a problem hiding this comment.
Yes. I tried use arrayList to reproduce the bug, but failed when there is only add operation. When I add remove operation in ut, it can reproduce similar problem.
Now the question become how user trigger the bug. It may be user is traversing the list, at the same time , he add processor to the list. Is it possible? or we can do something else to forbid it ? (I think it is not right to add processor when other processors are working on connections.It may cause different behavior in different connections. )
No description provided.