-
Notifications
You must be signed in to change notification settings - Fork 582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change to address NPE that may occur when multiple cdi-injected servlets #1485
Change to address NPE that may occur when multiple cdi-injected servlets #1485
Conversation
Code analysis and actionsDO NOT DELETE THIS COMMENT.
|
#build |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_Sv6xsPJfEee7FMdg7u8gzA Target locations of links might be accessible only to IBM employees. |
1 similar comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks good, but some minor suggested changes.
ModuleMetaData mmd = cmd.getModuleMetaData(); | ||
j2eeName = mmd.getJ2EEName(); | ||
} | ||
return Integer.toString(customizer.hashCode()) + j2eeName;// + ":" + Thread.currentThread().getId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remove the commented code: // + ":" + Thread.currentThread().getId();
@@ -69,7 +69,8 @@ | |||
private final static TraceComponent tc = Tr.register(LibertyJaxRsServerFactoryBean.class); | |||
|
|||
private final List<JaxRsFactoryBeanCustomizer> beanCustomizers = new LinkedList<JaxRsFactoryBeanCustomizer>(); | |||
private final Map<String, Object> beanCustomizerContexts = new HashMap<String, Object>(); | |||
// private final Map<String, Object> beanCustomizerContexts = new HashMap<String, Object>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this comment: // private final Map<String, Object> beanCustomizerContexts = new HashMap<String, Object>();
BeanCustomizerContext context = new BeanCustomizerContext(endpointInfo, moduleMetadata, cxfPRHolder); | ||
|
||
if (key != null && (oldCustomizerContext instanceof HashMap)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be better if it checked for an instance of Map
as opposed to HashMap
in case we decide to use different types of Maps in the future - I'm not sure, but I think we currently only use HashMaps (and subclasses of HashMaps - like ThreadBasedHashMap), so this is probably safe as-is, but would be safer to change to check for Map
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose HashMap in order to use the Map we were using... but I see your point for future flexibility. I'll change this.
The build jim-krueger-1485-20180105-2238 |
are specified in the web.xml for a JAXRS application with load-on-startup specified
6cd4904
to
b6afe46
Compare
#build |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_JNXv8PR5EeeSWo-prsQAeQ Target locations of links might be accessible only to IBM employees. |
The build jim-krueger-1485-20180108-1446 |
are specified in the web.xml for a JAXRS application with
load-on-startup specified