You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beehive.apache.org by Eddie O'Neil <ek...@bea.com> on 2005/05/31 21:41:43 UTC
v1/m1 and fixing InputStream leak -- was: Re: [jira] Resolved: (BEEHIVE-772)
service control leaks an InputStream
Yeah, I agree that it'd be good to fix...leaking streams doesn't tend
to scale well.
:)
I've pasted the diff below; please give this a review -- wouldn't be
good to break something at this late date.
Eddie
::::: diff.txt
Index:
src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
===================================================================
---
src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
(rev
ision 179055)
+++
src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
(wor
king copy)
@@ -454,14 +454,21 @@
if (wsdl != null) {
logger.debug("read wsdl from: " + wsdl.path());
- InputStream wsdlStream = getWSDLStream(wsdl.path());
+ InputStream wsdlStream = null;
+ try {
+ wsdlStream = getWSDLStream(wsdl.path());
- if (wsdlStream != null) {
- wsdlProcessor = new
XmlBeanWSDLProcessor(wsdlStream);
- } else {
- throw new RuntimeException(
+ if (wsdlStream != null) {
+ wsdlProcessor = new XmlBeanWSDLProcessor(wsdlStream);
+ } else {
+ throw new RuntimeException(
"No WSDL found at the
provided path: " + wsdl.path()
);
- }
+ }
+ }
+ finally {
+ if(wsdlStream != null)
+ wsdlStream.close();
+ }
} else {
throw new RuntimeException("No WSDL annotation
found.");
}
:::::
Richard Feit wrote:
> I think that leaking streams is bad enough that the fix should go into
> the branch. The fix (minus the code cleanup in the same checkin) looks
> straightforward...
>
> Eddie O'Neil (JIRA) wrote:
>
>> [ http://issues.apache.org/jira/browse/BEEHIVE-772?page=all ]
>> Eddie O'Neil resolved BEEHIVE-772:
>> ----------------------------------
>>
>> Assign To: (was: Eddie O'Neil)
>> Resolution: Fixed
>>
>> Fixed in trunk/; I've not made this change to 1.0m1. If anyone
>> believes it should go up there (I sort of feel that way), let me know
>> and I'll be happy to integ it.
>>
>>
>>
>>> service control leaks an InputStream
>>> ------------------------------------
>>>
>>> Key: BEEHIVE-772
>>> URL: http://issues.apache.org/jira/browse/BEEHIVE-772
>>> Project: Beehive
>>> Type: Bug
>>> Components: Controls
>>> Versions: V1Alpha, V1Beta, V1
>>> Reporter: Eddie O'Neil
>>> Priority: Critical
>>> Fix For: TBD
>>>
>>
>>
>>
>>
>>> The current ServiceControlImpl.jcs loads a WSDL InputStream but never
>>> closes that stream. Thus, we're leaking streams.
>>> Fix coming shortly.
>>>
>>
>>
>>
>>
>
Re: v1/m1 and fixing InputStream leak -- was: Re: [jira] Resolved:
(BEEHIVE-772) service control leaks an InputStream
Posted by Richard Feit <ri...@bea.com>.
Great, thanks; seems good to me. I say fire away!
Rich
Eddie O'Neil wrote:
>
> Sure; that seems reasonable. The InputStream is used just to load
> the XMLBean, and I don't believe that the InputStream needs to remain
> open after the bean's document is constructed.
>
> But, given the late date, I don't mind erring on the side of
> caution...this will get cleaned up in trunk/ soon-ish.
>
> Diff below -- this is uglier because I moved the null check on wsdl
> up and nested the rest of the method in try / finally.
>
> Eddie
>
> ::::: diff.txt
> Index:
> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>
> ===================================================================
> ---
> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
> (rev
> ision 179055)
> +++
> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
> (wor
> king copy)
> @@ -447,47 +447,51 @@
> private synchronized void initialize() throws Exception {
> if (initialized)
> return;
> - XmlBeanWSDLProcessor wsdlProcessor;
> -
> ServiceControl.WSDL wsdl = (ServiceControl.WSDL)
> cbContext
>
> .getControlPropertySet(ServiceControl.WSDL.class);
>
> - if (wsdl != null) {
> - logger.debug("read wsdl from: " + wsdl.path());
> - InputStream wsdlStream =
> getWSDLStream(wsdl.path());
> + if(wsdl == null)
> + throw new RuntimeException("No WSDL annotation
> found.");
> +
> + logger.debug("read wsdl from: " + wsdl.path());
> + InputStream wsdlStream = null;
> + try {
> + wsdlStream = getWSDLStream(wsdl.path());
>
> - if (wsdlStream != null) {
> - wsdlProcessor = new
> XmlBeanWSDLProcessor(wsdlStream);
> - } else {
> - throw new RuntimeException(
> - "No WSDL found at the
> provided path: " + wsdl.path()
> );
> + XmlBeanWSDLProcessor wsdlProcessor;
> + if (wsdlStream != null) {
> + wsdlProcessor = new XmlBeanWSDLProcessor(wsdlStream);
> + } else {
> + throw new RuntimeException(
> + "No WSDL found at the provided path: " +
> wsdl.path());
> }
> - } else {
> - throw new RuntimeException("No WSDL annotation
> found.");
> - }
>
> - ServiceFactory factory = ServiceFactory.newInstance();
> + ServiceFactory factory = ServiceFactory.newInstance();
>
> - service =
> factory.createService(wsdlProcessor.getServiceName());
> + service =
> factory.createService(wsdlProcessor.getServiceName());
>
> - HandlerInfo hInfo = new HandlerInfo();
> - hInfo.setHandlerClass(HeaderHandler.class);
> + HandlerInfo hInfo = new HandlerInfo();
> + hInfo.setHandlerClass(HeaderHandler.class);
>
> - TypeMapping tm = service.getTypeMappingRegistry()
> - .getDefaultTypeMapping();
> - lookupService = new SystemTypeLookupService();
> - registrar = new AxisTypeRegistrar(
> - (org.apache.axis.encoding.TypeMapping)
> tm, lookupService);
> - configureEndPoint();
> + TypeMapping tm = service.getTypeMappingRegistry()
> + .getDefaultTypeMapping();
> + lookupService = new SystemTypeLookupService();
> + registrar = new AxisTypeRegistrar(
> + (org.apache.axis.encoding.TypeMapping) tm, lookupService);
> + configureEndPoint();
>
> - mWSTM = wsdlProcessor.getObjectModel(lookupService);
> + mWSTM = wsdlProcessor.getObjectModel(lookupService);
>
> - portType = new QName(mWSTM.getWsTargetNamespace(),
> mWSTM.getWsName()); // porttype
> - // name
> - service.getHandlerRegistry().getHandlerChain(portType).add(hInfo);
> + portType = new QName(mWSTM.getWsTargetNamespace(),
> mWSTM.getWsName()); // portty
> pe
> + // name
> + service.getHandlerRegistry().getHandlerChain(portType).add(hInfo);
>
> - initialized = true;
> -
> + initialized = true;
> + }
> + finally {
> + if(wsdlStream != null)
> + wsdlStream.close();
> + }
> }
>
> private String getAlternateOperationName(Method method) {
> :::::
>
> Richard Feit wrote:
>
>> One question: could the finally block be moved to the end of the
>> method? It looks like XmlBeanWSDLProcessor only uses the stream in
>> methods called from its constructor, but it does seem like it would
>> be safer to avoid closing the stream before XmlBeanWSDLProcessor
>> could *possibly* access it.
>>
>>
>> Eddie O'Neil wrote:
>>
>>>
>>> Yeah, I agree that it'd be good to fix...leaking streams doesn't
>>> tend to scale well.
>>>
>>> :)
>>>
>>> I've pasted the diff below; please give this a review -- wouldn't
>>> be good to break something at this late date.
>>>
>>> Eddie
>>>
>>>
>>>
>>> ::::: diff.txt
>>> Index:
>>> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>>>
>>> ===================================================================
>>> ---
>>> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>>> (rev
>>> ision 179055)
>>> +++
>>> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>>> (wor
>>> king copy)
>>> @@ -454,14 +454,21 @@
>>>
>>> if (wsdl != null) {
>>> logger.debug("read wsdl from: " + wsdl.path());
>>> - InputStream wsdlStream =
>>> getWSDLStream(wsdl.path());
>>> + InputStream wsdlStream = null;
>>> + try {
>>> + wsdlStream = getWSDLStream(wsdl.path());
>>>
>>> - if (wsdlStream != null) {
>>> - wsdlProcessor = new
>>> XmlBeanWSDLProcessor(wsdlStream);
>>> - } else {
>>> - throw new RuntimeException(
>>> + if (wsdlStream != null) {
>>> + wsdlProcessor = new
>>> XmlBeanWSDLProcessor(wsdlStream);
>>> + } else {
>>> + throw new RuntimeException(
>>> "No WSDL found at
>>> the provided path: " + wsdl.path()
>>> );
>>> - }
>>> + }
>>> + }
>>> + finally {
>>> + if(wsdlStream != null)
>>> + wsdlStream.close();
>>> + }
>>> } else {
>>> throw new RuntimeException("No WSDL
>>> annotation found.");
>>> }
>>> :::::
>>>
>>> Richard Feit wrote:
>>>
>>>> I think that leaking streams is bad enough that the fix should go
>>>> into the branch. The fix (minus the code cleanup in the same
>>>> checkin) looks straightforward...
>>>>
>>>> Eddie O'Neil (JIRA) wrote:
>>>>
>>>>> [ http://issues.apache.org/jira/browse/BEEHIVE-772?page=all ]
>>>>> Eddie O'Neil resolved BEEHIVE-772:
>>>>> ----------------------------------
>>>>>
>>>>> Assign To: (was: Eddie O'Neil)
>>>>> Resolution: Fixed
>>>>>
>>>>> Fixed in trunk/; I've not made this change to 1.0m1. If anyone
>>>>> believes it should go up there (I sort of feel that way), let me
>>>>> know and I'll be happy to integ it.
>>>>>
>>>>>
>>>>>
>>>>>> service control leaks an InputStream
>>>>>> ------------------------------------
>>>>>>
>>>>>> Key: BEEHIVE-772
>>>>>> URL: http://issues.apache.org/jira/browse/BEEHIVE-772
>>>>>> Project: Beehive
>>>>>> Type: Bug
>>>>>> Components: Controls
>>>>>> Versions: V1Alpha, V1Beta, V1
>>>>>> Reporter: Eddie O'Neil
>>>>>> Priority: Critical
>>>>>> Fix For: TBD
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> The current ServiceControlImpl.jcs loads a WSDL InputStream but
>>>>>> never closes that stream. Thus, we're leaking streams.
>>>>>> Fix coming shortly.
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
Re: v1/m1 and fixing InputStream leak -- was: Re: [jira] Resolved:
(BEEHIVE-772) service control leaks an InputStream
Posted by Eddie O'Neil <ek...@bea.com>.
Sure; that seems reasonable. The InputStream is used just to load
the XMLBean, and I don't believe that the InputStream needs to remain
open after the bean's document is constructed.
But, given the late date, I don't mind erring on the side of
caution...this will get cleaned up in trunk/ soon-ish.
Diff below -- this is uglier because I moved the null check on wsdl
up and nested the rest of the method in try / finally.
Eddie
::::: diff.txt
Index:
src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
===================================================================
---
src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
(rev
ision 179055)
+++
src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
(wor
king copy)
@@ -447,47 +447,51 @@
private synchronized void initialize() throws Exception {
if (initialized)
return;
- XmlBeanWSDLProcessor wsdlProcessor;
-
ServiceControl.WSDL wsdl = (ServiceControl.WSDL) cbContext
.getControlPropertySet(ServiceControl.WSDL.class);
- if (wsdl != null) {
- logger.debug("read wsdl from: " + wsdl.path());
- InputStream wsdlStream = getWSDLStream(wsdl.path());
+ if(wsdl == null)
+ throw new RuntimeException("No WSDL annotation
found.");
+
+ logger.debug("read wsdl from: " + wsdl.path());
+ InputStream wsdlStream = null;
+ try {
+ wsdlStream = getWSDLStream(wsdl.path());
- if (wsdlStream != null) {
- wsdlProcessor = new
XmlBeanWSDLProcessor(wsdlStream);
- } else {
- throw new RuntimeException(
- "No WSDL found at the
provided path: " + wsdl.path()
);
+ XmlBeanWSDLProcessor wsdlProcessor;
+ if (wsdlStream != null) {
+ wsdlProcessor = new XmlBeanWSDLProcessor(wsdlStream);
+ } else {
+ throw new RuntimeException(
+ "No WSDL found at the provided path: " + wsdl.path());
}
- } else {
- throw new RuntimeException("No WSDL annotation
found.");
- }
- ServiceFactory factory = ServiceFactory.newInstance();
+ ServiceFactory factory = ServiceFactory.newInstance();
- service =
factory.createService(wsdlProcessor.getServiceName());
+ service =
factory.createService(wsdlProcessor.getServiceName());
- HandlerInfo hInfo = new HandlerInfo();
- hInfo.setHandlerClass(HeaderHandler.class);
+ HandlerInfo hInfo = new HandlerInfo();
+ hInfo.setHandlerClass(HeaderHandler.class);
- TypeMapping tm = service.getTypeMappingRegistry()
- .getDefaultTypeMapping();
- lookupService = new SystemTypeLookupService();
- registrar = new AxisTypeRegistrar(
- (org.apache.axis.encoding.TypeMapping)
tm, lookupService);
- configureEndPoint();
+ TypeMapping tm = service.getTypeMappingRegistry()
+ .getDefaultTypeMapping();
+ lookupService = new SystemTypeLookupService();
+ registrar = new AxisTypeRegistrar(
+
(org.apache.axis.encoding.TypeMapping) tm, lookupService);
+ configureEndPoint();
- mWSTM = wsdlProcessor.getObjectModel(lookupService);
+ mWSTM = wsdlProcessor.getObjectModel(lookupService);
- portType = new QName(mWSTM.getWsTargetNamespace(),
mWSTM.getWsName()); // porttype
- // name
-
service.getHandlerRegistry().getHandlerChain(portType).add(hInfo);
+ portType = new QName(mWSTM.getWsTargetNamespace(),
mWSTM.getWsName()); // portty
pe
+ // name
+
service.getHandlerRegistry().getHandlerChain(portType).add(hInfo);
- initialized = true;
-
+ initialized = true;
+ }
+ finally {
+ if(wsdlStream != null)
+ wsdlStream.close();
+ }
}
private String getAlternateOperationName(Method method) {
:::::
Richard Feit wrote:
> One question: could the finally block be moved to the end of the
> method? It looks like XmlBeanWSDLProcessor only uses the stream in
> methods called from its constructor, but it does seem like it would be
> safer to avoid closing the stream before XmlBeanWSDLProcessor could
> *possibly* access it.
>
>
> Eddie O'Neil wrote:
>
>>
>> Yeah, I agree that it'd be good to fix...leaking streams doesn't
>> tend to scale well.
>>
>> :)
>>
>> I've pasted the diff below; please give this a review -- wouldn't be
>> good to break something at this late date.
>>
>> Eddie
>>
>>
>>
>> ::::: diff.txt
>> Index:
>> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>>
>> ===================================================================
>> ---
>> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>> (rev
>> ision 179055)
>> +++
>> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>> (wor
>> king copy)
>> @@ -454,14 +454,21 @@
>>
>> if (wsdl != null) {
>> logger.debug("read wsdl from: " + wsdl.path());
>> - InputStream wsdlStream =
>> getWSDLStream(wsdl.path());
>> + InputStream wsdlStream = null;
>> + try {
>> + wsdlStream = getWSDLStream(wsdl.path());
>>
>> - if (wsdlStream != null) {
>> - wsdlProcessor = new
>> XmlBeanWSDLProcessor(wsdlStream);
>> - } else {
>> - throw new RuntimeException(
>> + if (wsdlStream != null) {
>> + wsdlProcessor = new
>> XmlBeanWSDLProcessor(wsdlStream);
>> + } else {
>> + throw new RuntimeException(
>> "No WSDL found at the
>> provided path: " + wsdl.path()
>> );
>> - }
>> + }
>> + }
>> + finally {
>> + if(wsdlStream != null)
>> + wsdlStream.close();
>> + }
>> } else {
>> throw new RuntimeException("No WSDL annotation
>> found.");
>> }
>> :::::
>>
>> Richard Feit wrote:
>>
>>> I think that leaking streams is bad enough that the fix should go
>>> into the branch. The fix (minus the code cleanup in the same
>>> checkin) looks straightforward...
>>>
>>> Eddie O'Neil (JIRA) wrote:
>>>
>>>> [ http://issues.apache.org/jira/browse/BEEHIVE-772?page=all ]
>>>> Eddie O'Neil resolved BEEHIVE-772:
>>>> ----------------------------------
>>>>
>>>> Assign To: (was: Eddie O'Neil)
>>>> Resolution: Fixed
>>>>
>>>> Fixed in trunk/; I've not made this change to 1.0m1. If anyone
>>>> believes it should go up there (I sort of feel that way), let me
>>>> know and I'll be happy to integ it.
>>>>
>>>>
>>>>
>>>>> service control leaks an InputStream
>>>>> ------------------------------------
>>>>>
>>>>> Key: BEEHIVE-772
>>>>> URL: http://issues.apache.org/jira/browse/BEEHIVE-772
>>>>> Project: Beehive
>>>>> Type: Bug
>>>>> Components: Controls
>>>>> Versions: V1Alpha, V1Beta, V1
>>>>> Reporter: Eddie O'Neil
>>>>> Priority: Critical
>>>>> Fix For: TBD
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> The current ServiceControlImpl.jcs loads a WSDL InputStream but
>>>>> never closes that stream. Thus, we're leaking streams.
>>>>> Fix coming shortly.
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>
Re: v1/m1 and fixing InputStream leak -- was: Re: [jira] Resolved:
(BEEHIVE-772) service control leaks an InputStream
Posted by Richard Feit <ri...@bea.com>.
One question: could the finally block be moved to the end of the
method? It looks like XmlBeanWSDLProcessor only uses the stream in
methods called from its constructor, but it does seem like it would be
safer to avoid closing the stream before XmlBeanWSDLProcessor could
*possibly* access it.
Eddie O'Neil wrote:
>
> Yeah, I agree that it'd be good to fix...leaking streams doesn't
> tend to scale well.
>
> :)
>
> I've pasted the diff below; please give this a review -- wouldn't be
> good to break something at this late date.
>
> Eddie
>
>
>
> ::::: diff.txt
> Index:
> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
>
> ===================================================================
> ---
> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
> (rev
> ision 179055)
> +++
> src/webservice/org/apache/beehive/controls/system/webservice/jaxrpc/ServiceControlImpl.jcs
> (wor
> king copy)
> @@ -454,14 +454,21 @@
>
> if (wsdl != null) {
> logger.debug("read wsdl from: " + wsdl.path());
> - InputStream wsdlStream =
> getWSDLStream(wsdl.path());
> + InputStream wsdlStream = null;
> + try {
> + wsdlStream = getWSDLStream(wsdl.path());
>
> - if (wsdlStream != null) {
> - wsdlProcessor = new
> XmlBeanWSDLProcessor(wsdlStream);
> - } else {
> - throw new RuntimeException(
> + if (wsdlStream != null) {
> + wsdlProcessor = new
> XmlBeanWSDLProcessor(wsdlStream);
> + } else {
> + throw new RuntimeException(
> "No WSDL found at the
> provided path: " + wsdl.path()
> );
> - }
> + }
> + }
> + finally {
> + if(wsdlStream != null)
> + wsdlStream.close();
> + }
> } else {
> throw new RuntimeException("No WSDL annotation
> found.");
> }
> :::::
>
> Richard Feit wrote:
>
>> I think that leaking streams is bad enough that the fix should go
>> into the branch. The fix (minus the code cleanup in the same
>> checkin) looks straightforward...
>>
>> Eddie O'Neil (JIRA) wrote:
>>
>>> [ http://issues.apache.org/jira/browse/BEEHIVE-772?page=all ]
>>> Eddie O'Neil resolved BEEHIVE-772:
>>> ----------------------------------
>>>
>>> Assign To: (was: Eddie O'Neil)
>>> Resolution: Fixed
>>>
>>> Fixed in trunk/; I've not made this change to 1.0m1. If anyone
>>> believes it should go up there (I sort of feel that way), let me
>>> know and I'll be happy to integ it.
>>>
>>>
>>>
>>>> service control leaks an InputStream
>>>> ------------------------------------
>>>>
>>>> Key: BEEHIVE-772
>>>> URL: http://issues.apache.org/jira/browse/BEEHIVE-772
>>>> Project: Beehive
>>>> Type: Bug
>>>> Components: Controls
>>>> Versions: V1Alpha, V1Beta, V1
>>>> Reporter: Eddie O'Neil
>>>> Priority: Critical
>>>> Fix For: TBD
>>>>
>>>
>>>
>>>
>>>
>>>
>>>> The current ServiceControlImpl.jcs loads a WSDL InputStream but
>>>> never closes that stream. Thus, we're leaking streams.
>>>> Fix coming shortly.
>>>>
>>>
>>>
>>>
>>>
>>>
>>
>
>