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.
