You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "JerryChin (GitHub)" <gi...@apache.org> on 2019/03/11 04:29:26 UTC
[GitHub] [incubator-dubbo] JerryChin opened issue #3629:
ServiceConfig duplicate export produces irrelevant exception
I mistakenly exported `ServiceConfig` twice, the second call produces the following exception which makes no sense:
```
java.lang.reflect.InvocationTargetException: null
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_91]
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_91]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_91]
at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_91]
at org.apache.dubbo.config.AbstractConfig.refresh(AbstractConfig.java:568) [dubbo-2.7.0.jar:2.7.0]
at org.apache.dubbo.config.ServiceConfig.checkAndUpdateSubConfigs(ServiceConfig.java:272) [dubbo-2.7.0.jar:2.7.0]
at org.apache.dubbo.config.ServiceConfig.export(ServiceConfig.java:328) [dubbo-2.7.0.jar:2.7.0]
at xx.xx.xx.xx.rpc.NodeRpcServerV3.start(NodeRpcServerV3.java:298) [classes/:?]
at xx.xx.xx.xx.rpc.NodeRpcV3Test.init(NodeRpcV3Test.java:21) [test-classes/:?]
....
Caused by: java.lang.IllegalArgumentException: Unsupported generic type false
at org.apache.dubbo.config.ServiceConfig.setGeneric(ServiceConfig.java:959) ~[dubbo-2.7.0.jar:2.7.0]
... 67 more
```
Dubbo version: 2.7.0
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] kexianjun commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "kexianjun (GitHub)" <gi...@apache.org>.
> @kexianjun
>
> I do have some ideas after reviewing the source code of `ServiceConfig`.
>
> First, the existing implementation by using two variables for controlling `ServiceConfig` exporting state is not only awkward to code, but also can theoretically lead invalid state transitions (`exported` and `export` both simultaneously true or false).
>
> Secondly, the `exported` is set in `doExport()` method which could be called asynchronously, this makes it hard to refactor to prevent duplication exporting.
>
> Okay, there's not "last but not least".
>
> I'll suggest to introduce the following changes:
>
> 1. refactor the two variables aforementioned into one `AtomicInteger` variable.
> 2. `compare and set` exporting state in `export()` and `unexport()` methods **before doing anything**.
> 3. it's obviously to see that we can now safely remove the `synchronized` from the two methods.
could you pls send a pr about your suggestion?
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] JerryChin commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "JerryChin (GitHub)" <gi...@apache.org>.
I do have some ideas after reviewing the source code of `ServiceConfig`.
First, the existing implementation by using two variables for controlling `ServiceConfig` exporting state is not only awkward to code, but also can theoretically lead invalid state transition (`exported` and `export` both simultaneously true or false).
Secondly, the `exported` is set in `doExport()` method which could be called asynchronously, which make it hard to refactor to prevent duplication exporting.
Okay, there's not "last but not least".
I'll suggest to refactor:
1. the two variables aforementioned into one `AtomicInteger` variable.
2. `compare and set` exporting state in `export()` and `unexport()` methods `before doing anything`.
3. it's obviously to see that we can now safely remove the `synchronized` from the two methods.
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] JerryChin commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "JerryChin (GitHub)" <gi...@apache.org>.
@kexianjun
I do have some ideas after reviewing the source code of `ServiceConfig`.
First, the existing implementation by using two variables for controlling `ServiceConfig` exporting state is not only awkward to code, but also can theoretically lead invalid state transitions (`exported` and `export` both simultaneously true or false).
Secondly, the `exported` is set in `doExport()` method which could be called asynchronously, this makes it hard to refactor to prevent duplication exporting.
Okay, there's not "last but not least".
I'll suggest to introduce the following changes:
1. the two variables aforementioned into one `AtomicInteger` variable.
2. `compare and set` exporting state in `export()` and `unexport()` methods **before doing anything**.
3. it's obviously to see that we can now safely remove the `synchronized` from the two methods.
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] kexianjun commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "kexianjun (GitHub)" <gi...@apache.org>.
sure this exception is misleading,do you have any idea to improve this?
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] JerryChin commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "JerryChin (GitHub)" <gi...@apache.org>.
@kexianjun
I do have some ideas after reviewing the source code of `ServiceConfig`.
First, the existing implementation by using two variables for controlling `ServiceConfig` exporting state is not only awkward to code, but also can theoretically lead invalid state transitions (`exported` and `export` both simultaneously true or false).
Secondly, the `exported` is set in `doExport()` method which could be called asynchronously, this makes it hard to refactor to prevent duplication exporting.
Okay, there's not "last but not least".
I'll suggest to introduce the following changes:
1. refactor the two variables aforementioned into one `AtomicInteger` variable.
2. `compare and set` exporting state in `export()` and `unexport()` methods **before doing anything**.
3. it's obviously to see that we can now safely remove the `synchronized` from the two methods.
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] JerryChin commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "JerryChin (GitHub)" <gi...@apache.org>.
I'll try to work out a PR for this issue this evening. @kexianjun
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] JerryChin commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "JerryChin (GitHub)" <gi...@apache.org>.
@kexianjun
I do have some ideas after reviewing the source code of `ServiceConfig`.
First, the existing implementation by using two variables for controlling `ServiceConfig` exporting state is not only awkward to code, but also can theoretically lead invalid state transition (`exported` and `export` both simultaneously true or false).
Secondly, the `exported` is set in `doExport()` method which could be called asynchronously, which make it hard to refactor to prevent duplication exporting.
Okay, there's not "last but not least".
I'll suggest to refactor:
1. the two variables aforementioned into one `AtomicInteger` variable.
2. `compare and set` exporting state in `export()` and `unexport()` methods `before doing anything`.
3. it's obviously to see that we can now safely remove the `synchronized` from the two methods.
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] JerryChin commented on issue #3629:
ServiceConfig duplicate export produces irrelevant exception
Posted by "JerryChin (GitHub)" <gi...@apache.org>.
@kexianjun
I do have some ideas after reviewing the source code of `ServiceConfig`.
First, the existing implementation by using two variables for controlling `ServiceConfig` exporting state is not only awkward to code, but also can theoretically lead invalid state transitions (`exported` and `export` both simultaneously true or false).
Secondly, the `exported` is set in `doExport()` method which could be called asynchronously, this makes it hard to refactor to prevent duplication exporting.
Okay, there's not "last but not least".
I'll suggest to introduce the following changes:
1. the two variables aforementioned into one `AtomicInteger` variable.
2. `compare and set` exporting state in `export()` and `unexport()` methods `before doing anything`.
3. it's obviously to see that we can now safely remove the `synchronized` from the two methods.
[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3629 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org