Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
0027877Openbravo ERPA. Platformpublic2014-10-15 14:372014-12-30 23:26
shuehner 
NaroaIriarte 
normalmajorhave not tried
closedfixed 
5
 
3.0PR15Q1 
alostale
Core
No
0027877: Possible connection leak in UsageAudit.java in error case
Code in UsageAudit.java

in the last function of that file.

Is doing getTransactionConnection which opens a new db connection.
And does 1 insert + releaseCommitconnection.

However in case of errors as those 3 catch blocks no releaeRollbackConnection is called.

That means that in those error cases the open connections i never closed.

That case is unlikely to trigger as that code is wrong as it is a long time.

However the effect will be very bad (unbounded db connection leak).
so reporting as major.
-
No tags attached.
related to defect 0027878 closed marvintm Code in DalConnectionprovider relate to getTransactionConnection seems to leak db connections always 
Issue History
2014-10-15 14:37shuehnerNew Issue
2014-10-15 14:37shuehnerAssigned To => AugustoMauch
2014-10-15 14:37shuehnerModules => Core
2014-10-15 14:37shuehnerTriggers an Emergency Pack => No
2014-10-15 14:41shuehnerRelationship addedrelated to 0027878
2014-11-28 09:20alostaleAssigned ToAugustoMauch => NaroaIriarte
2014-12-01 13:48NaroaIriarteNote Added: 0072194
2014-12-01 14:09NaroaIriarteNote Edited: 0072194bug_revision_view_page.php?bugnote_id=0072194#r7194
2014-12-01 14:13NaroaIriarteNote Added: 0072196
2014-12-03 09:21hgbotCheckin
2014-12-03 09:21hgbotNote Added: 0072257
2014-12-03 09:21hgbotStatusnew => resolved
2014-12-03 09:21hgbotResolutionopen => fixed
2014-12-03 09:21hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/1643c8fecb96f82ac652260a49cffdf21f497320 [^]
2014-12-03 09:25alostaleReview Assigned To => alostale
2014-12-03 09:25alostaleNote Added: 0072258
2014-12-03 09:25alostaleStatusresolved => closed
2014-12-03 09:25alostaleFixed in Version => 3.0PR15Q1
2014-12-30 23:26hudsonbotCheckin
2014-12-30 23:26hudsonbotNote Added: 0073166

Notes
(0072194)
NaroaIriarte   
2014-12-01 13:48   
(edited on: 2014-12-01 14:09)
-An exception has been forced to watch if it is true that connection leak exists in case of error. But it seems not to occur.
For testing this, the first step is to throw an exception to force to execute the code of the exceptions. The second step is to log in and end session as many times as wanted and after this, the last step, making a postgres query to look the connexions. After this test it seems not to exist any connection leak.
Anyway the releaseRollbackConnection has been added to the code, because it seems to be better this way.

(0072196)
NaroaIriarte   
2014-12-01 14:13   
Some tests have been performed:

-Firstly, an exception has been thrown for forcing to execute the new added code for exceptions. The code is reached correctly so it works as expected.
-A postgres query has been launched to check the connections to the data base.
(0072257)
hgbot   
2014-12-03 09:21   
Repository: erp/devel/pi
Changeset: 1643c8fecb96f82ac652260a49cffdf21f497320
Author: Naroa Iriarte <naroa.iriarte <at> openbravo.com>
Date: Mon Dec 01 15:22:06 2014 +0100
URL: http://code.openbravo.com/erp/devel/pi/rev/1643c8fecb96f82ac652260a49cffdf21f497320 [^]

Fixed issue 27877: No rollback is called in connection error catch.

It seemed to be possible to have a connection leak caused in the "UsageAudit.java" class.
That was because at the final function of this class in a try catch block there was no rollback done.
After some tests it seems not to be any connection leak even without the rollback. But the proper code
to make the rollback has been created, because it seems to be better that way.
For developing this, a new method has been created "releaseConnection". This method is called in the three
try catch blocks of the method "auditActionNoDal".
Now, if one of those excepcions are reached, a rollback will be done.

---
M src/org/openbravo/erpCommon/security/UsageAudit.java
---
(0072258)
alostale   
2014-12-03 09:25   
code reviewed

tested forcing execption in the code. Note before the fix is applied, the problem was not "only" leaked connections; if the exception occurred after the insert, uncommited transactions trying to insert elements in ad_session_usage_audit were kept forever; they could cause locks in ad_session due to FK resulting in completely stalling the whole session.
(0073166)
hudsonbot   
2014-12-30 23:26   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/6525fe229e06 [^]
Maturity status: Test