[jira] [Commented] (FELIX-5199) Race condition in HttpServiceFactory.getService() causing exception
[
https://issues.apache.org/jira/browse/FELIX-5199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15206779#comment-15206779
]
David Bosschaert commented on FELIX-5199:
-
Yeah, when I looked at it the best solution seemed to split this object into
two separate objects, but I didn't get to implement that yet...
> Race condition in HttpServiceFactory.getService() causing exception
> ---
>
> Key: FELIX-5199
> URL: https://issues.apache.org/jira/browse/FELIX-5199
> Project: Felix
> Issue Type: Bug
> Components: HTTP Service
>Affects Versions: http.base-3.0.6
>Reporter: David Bosschaert
>Assignee: David Bosschaert
> Fix For: http.base-3.0.8
>
> Attachments: felix-5199.patch
>
>
> The HttpServiceFactory.getService() is as follows:
> {code}
> public HttpService getService(final Bundle bundle, final
> ServiceRegistration reg)
> {
> final ServletContext servletContext = this.context;
> if ( servletContext != null ) {
> return new PerBundleHttpServiceImpl(bundle,
> this.sharedHttpService,
> this.context,
> this.contextAttributeListenerManager,
> this.sharedContextAttributes,
> this.requestListenerManager,
> this.requestAttributeListenerManager);
> }
> return null;
> }{code}
> However it is possible that this.context is set to {{null}} after the check
> for {{null}} is done but before the constructor is called causing a null
> servlet context to be passed to {{PerBundleHttpServiceImpl}}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FELIX-5199) Race condition in HttpServiceFactory.getService() causing exception
[
https://issues.apache.org/jira/browse/FELIX-5199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15203515#comment-15203515
]
Anders Engström commented on FELIX-5199:
While investigating FELIX-5219 I ran into some issues that I believe is related
to this patch.
If a {{ServiceTracker}} is opened and listening for {{HttpService}} (*before*
{{HttpServiceFactory#start}} have been called) the {{active}} flag in
{{HttpServiceFactory#getService}} will never be {{!= null}} (and the returned
service will be {{null}}.
The reason is that the ServiceTracker will get a callback to {{addingService}}
synchronously when the service-factory is registered (line 114 in
{{HttpServiceFactory}}. At this point the {{active}} flag is still {{false}}
(it's set to {{true}} right after the registration. If the ServiceTracker tries
to resolve the service-reference in its {{addingService}} this will call into
{{HttpServiceFactory#getService}} - but {{active}} is false, so no service is
returned.
I tried setting {{active}} to true just before registering the
HttpServiceFactory, and this seemed to work - but perhaps that will lead to
similar race conditions as in the original code before this patch?
> Race condition in HttpServiceFactory.getService() causing exception
> ---
>
> Key: FELIX-5199
> URL: https://issues.apache.org/jira/browse/FELIX-5199
> Project: Felix
> Issue Type: Bug
> Components: HTTP Service
>Affects Versions: http.base-3.0.6
>Reporter: David Bosschaert
>Assignee: David Bosschaert
> Fix For: http.base-3.0.8
>
> Attachments: felix-5199.patch
>
>
> The HttpServiceFactory.getService() is as follows:
> {code}
> public HttpService getService(final Bundle bundle, final
> ServiceRegistration reg)
> {
> final ServletContext servletContext = this.context;
> if ( servletContext != null ) {
> return new PerBundleHttpServiceImpl(bundle,
> this.sharedHttpService,
> this.context,
> this.contextAttributeListenerManager,
> this.sharedContextAttributes,
> this.requestListenerManager,
> this.requestAttributeListenerManager);
> }
> return null;
> }{code}
> However it is possible that this.context is set to {{null}} after the check
> for {{null}} is done but before the constructor is called causing a null
> servlet context to be passed to {{PerBundleHttpServiceImpl}}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FELIX-5199) Race condition in HttpServiceFactory.getService() causing exception
[
https://issues.apache.org/jira/browse/FELIX-5199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15168817#comment-15168817
]
David Bosschaert commented on FELIX-5199:
-
I improved the thread-safety in the fix after feedback from [~cziegeler] in
this commit: https://svn.apache.org/viewvc?view=revision=1732453
> Race condition in HttpServiceFactory.getService() causing exception
> ---
>
> Key: FELIX-5199
> URL: https://issues.apache.org/jira/browse/FELIX-5199
> Project: Felix
> Issue Type: Bug
> Components: HTTP Service
>Affects Versions: http.base-3.0.6
>Reporter: David Bosschaert
>Assignee: David Bosschaert
> Fix For: http.base-3.0.8
>
> Attachments: felix-5199.patch
>
>
> The HttpServiceFactory.getService() is as follows:
> {code}
> public HttpService getService(final Bundle bundle, final
> ServiceRegistration reg)
> {
> final ServletContext servletContext = this.context;
> if ( servletContext != null ) {
> return new PerBundleHttpServiceImpl(bundle,
> this.sharedHttpService,
> this.context,
> this.contextAttributeListenerManager,
> this.sharedContextAttributes,
> this.requestListenerManager,
> this.requestAttributeListenerManager);
> }
> return null;
> }{code}
> However it is possible that this.context is set to {{null}} after the check
> for {{null}} is done but before the constructor is called causing a null
> servlet context to be passed to {{PerBundleHttpServiceImpl}}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FELIX-5199) Race condition in HttpServiceFactory.getService() causing exception
[
https://issues.apache.org/jira/browse/FELIX-5199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15168648#comment-15168648
]
David Bosschaert commented on FELIX-5199:
-
Thanks for the feedback, [~cziegeler]. I didn't think of checking for the other
objects as {{PerBundleHttpServiceImpl}} only seems to require the Bundle and
the ServletContext to be non-null. As this is a service factory which is called
by the framework the Bundle can never be null...
I agree that a started/stopped flag would cover the overall case. Let me rework
the patch for that...
> Race condition in HttpServiceFactory.getService() causing exception
> ---
>
> Key: FELIX-5199
> URL: https://issues.apache.org/jira/browse/FELIX-5199
> Project: Felix
> Issue Type: Bug
> Components: HTTP Service
>Affects Versions: http.base-3.0.6
>Reporter: David Bosschaert
>Assignee: David Bosschaert
> Attachments: felix-5199.patch
>
>
> The HttpServiceFactory.getService() is as follows:
> {code}
> public HttpService getService(final Bundle bundle, final
> ServiceRegistration reg)
> {
> final ServletContext servletContext = this.context;
> if ( servletContext != null ) {
> return new PerBundleHttpServiceImpl(bundle,
> this.sharedHttpService,
> this.context,
> this.contextAttributeListenerManager,
> this.sharedContextAttributes,
> this.requestListenerManager,
> this.requestAttributeListenerManager);
> }
> return null;
> }{code}
> However it is possible that this.context is set to {{null}} after the check
> for {{null}} is done but before the constructor is called causing a null
> servlet context to be passed to {{PerBundleHttpServiceImpl}}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FELIX-5199) Race condition in HttpServiceFactory.getService() causing exception
[
https://issues.apache.org/jira/browse/FELIX-5199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15168580#comment-15168580
]
Carsten Ziegeler commented on FELIX-5199:
-
I think we need to check all objects that can get invalidated in stop(). If any
of these is null or closed, we should not return the service anymore. So maybe
using a simple flag (started/stopped) that's set in start/stop and evaluated in
the getService method is safer
> Race condition in HttpServiceFactory.getService() causing exception
> ---
>
> Key: FELIX-5199
> URL: https://issues.apache.org/jira/browse/FELIX-5199
> Project: Felix
> Issue Type: Bug
> Components: HTTP Service
>Affects Versions: http.base-3.0.6
>Reporter: David Bosschaert
>Assignee: David Bosschaert
> Attachments: felix-5199.patch
>
>
> The HttpServiceFactory.getService() is as follows:
> {code}
> public HttpService getService(final Bundle bundle, final
> ServiceRegistration reg)
> {
> final ServletContext servletContext = this.context;
> if ( servletContext != null ) {
> return new PerBundleHttpServiceImpl(bundle,
> this.sharedHttpService,
> this.context,
> this.contextAttributeListenerManager,
> this.sharedContextAttributes,
> this.requestListenerManager,
> this.requestAttributeListenerManager);
> }
> return null;
> }{code}
> However it is possible that this.context is set to {{null}} after the check
> for {{null}} is done but before the constructor is called causing a null
> servlet context to be passed to {{PerBundleHttpServiceImpl}}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
