Hi Jacques, The resources are not properly closed in your commit. It would take time to explain, suffice to say that your code leads to possible memory leaks. That is why I suggest a full review and carefully applying the stream memory management. There are multiple ways to fix this, some related to Java 7, but it would take time to explain, and it is your initiative so I leave it up to you to fix it.
Regards, Taher Alkhateeb On Fri, Sep 2, 2016 at 8:37 PM, Jacques Le Roux < [email protected]> wrote: > Why should I not close the Socket once the DataInputStream derived from it > has been used? > > In the case of an exception occurs will not the resource be leaked? > > Do you mean that I should also close the DataInputStream before closing > the Socket? > > ie something like > > Index: applications/accounting/src/main/java/org/apache/ofbiz/accou > nting/thirdparty/gosoftware/PcChargeApi.java > =================================================================== > --- applications/accounting/src/main/java/org/apache/ofbiz/accou > nting/thirdparty/gosoftware/PcChargeApi.java (revision 1758990) > +++ applications/accounting/src/main/java/org/apache/ofbiz/accou > nting/thirdparty/gosoftware/PcChargeApi.java (working copy) > @@ -200,14 +200,17 @@ > try { > outDoc = UtilXml.readXmlDocument(buf.toString(), false); > } catch (ParserConfigurationException e) { > + dis.close(); > sock.close(); > throw new GeneralException(e); > } catch (SAXException e) { > + dis.close(); > sock.close(); > throw new GeneralException(e); > } > > PcChargeApi out = new PcChargeApi(outDoc); > + dis.close(); > sock.close(); > return out; > } else { > > Jacques > > > > Le 02/09/2016 à 19:16, Taher Alkhateeb a écrit : > >> Jacques this is still problematic, you are closing the streams upon >> exceptions! now all kinds of memory leaks and side effects could take >> place >> because you are not ensuring closing all streams properly. >> >> This needs a revert and a full review to make things tight in terms of >> memory. >> >> On Fri, Sep 2, 2016 at 8:08 PM, Jacques Le Roux < >> [email protected]> wrote: >> >> Le 02/09/2016 à 18:46, Jacopo Cappellato a écrit : >>> >>> On Fri, Sep 2, 2016 at 3:19 PM, Jacques Le Roux < >>>> [email protected]> wrote: >>>> >>>> Le 02/09/2016 à 12:30, Jacopo Cappellato a écrit : >>>> >>>>> ... >>>>> 2) I suspect that you are closing the socket connection too early >>>>> >>>>> in PcChargeApi >>>>>> >>>>>> I see no problem with this point, the "sock" socket connection is not >>>>>> >>>>> used >>>>> in code below. >>>>> ... >>>>> Jacques >>>>> >>>>> >>>>> Jacques, >>>>> >>>> at the moment I don't have time to explain but after your response >>>> above I >>>> am a bit concerned about the effort that you are carrying on, because >>>> you >>>> may break other code without even suspecting it. >>>> I have to go now, if others can explain then great, otherwise I will >>>> follow >>>> up with you asap. >>>> >>>> Jacopo >>>> >>>> Ah I think I got your point when back, I should better close the Socket >>>> >>> later, done at r1758990 >>> >>> Jacques >>> >>> >>> >
