Skip to content

Commit

Permalink
Merge pull request zaproxy#2430 from thc202/mem-leak-db-listeners
Browse files Browse the repository at this point in the history
Fix memory leak on session creation/loading
  • Loading branch information
psiinon committed Apr 25, 2016
2 parents 82b82d7 + 3b9e955 commit 9c48c81
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 172 deletions.
13 changes: 7 additions & 6 deletions src/org/parosproxy/paros/control/Control.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
// ZAP: 2015/09/17 Issue 1914: Support multiple add-on directories
// ZAP: 2015/11/04 Issue 1920: Report the host:port ZAP is listening on in daemon mode, or exit if it cant
// ZAP: 2016/03/23 Issue 2331: Custom Context Panels not show in existing contexts after installation of add-on
// ZAP: 2016/04/22 Issue 2428: Memory leak on session creation/loading

package org.parosproxy.paros.control;

Expand Down Expand Up @@ -327,8 +328,8 @@ public void run() {
}

log.info("New session file created");
control.getExtensionLoader().sessionChangedAllPlugin(session);
control.getExtensionLoader().databaseOpen(model.getDb());
control.getExtensionLoader().sessionChangedAllPlugin(session);
}

public void runCommandLineOpenSession(String fileName) throws Exception {
Expand All @@ -338,8 +339,8 @@ public void runCommandLineOpenSession(String fileName) throws Exception {
Session session = Model.getSingleton().getSession();
Model.getSingleton().openSession(fileName);
log.info("Session file opened");
control.getExtensionLoader().sessionChangedAllPlugin(session);
control.getExtensionLoader().databaseOpen(model.getDb());
control.getExtensionLoader().sessionChangedAllPlugin(session);
}

public void setExcludeFromProxyUrls(List<String> urls) {
Expand All @@ -366,8 +367,8 @@ public Session newSession() {
log.info("New Session");
getExtensionLoader().sessionAboutToChangeAllPlugin(null);
final Session session = model.newSession();
getExtensionLoader().sessionChangedAllPlugin(session);
getExtensionLoader().databaseOpen(model.getDb());
getExtensionLoader().sessionChangedAllPlugin(session);

if (View.isInitialised()) {
SwingUtilities.invokeLater(new Runnable() {
Expand Down Expand Up @@ -431,14 +432,14 @@ public void createAndOpenUntitledDb() throws ClassNotFoundException, Exception {
getExtensionLoader().sessionAboutToChangeAllPlugin(null);
model.closeSession();
model.createAndOpenUntitledDb();
getExtensionLoader().sessionChangedAllPlugin(model.getSession());
getExtensionLoader().databaseOpen(model.getDb());
getExtensionLoader().sessionChangedAllPlugin(model.getSession());
}

@Override
public void sessionOpened(File file, Exception e) {
getExtensionLoader().sessionChangedAllPlugin(model.getSession());
getExtensionLoader().databaseOpen(model.getDb());
getExtensionLoader().sessionChangedAllPlugin(model.getSession());
if (lastCallback != null) {
lastCallback.sessionOpened(file, e);
lastCallback = null;
Expand All @@ -448,8 +449,8 @@ public void sessionOpened(File file, Exception e) {

@Override
public void sessionSaved(Exception e) {
getExtensionLoader().sessionChangedAllPlugin(model.getSession());
getExtensionLoader().databaseOpen(model.getDb());
getExtensionLoader().sessionChangedAllPlugin(model.getSession());
if (lastCallback != null) {
lastCallback.sessionSaved(e);
lastCallback = null;
Expand Down
114 changes: 114 additions & 0 deletions src/org/parosproxy/paros/db/AbstractDatabase.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Zed Attack Proxy (ZAP) and its related class files.
*
* ZAP is an HTTP/HTTPS proxy for assessing web application security.
*
* Copyright 2016 The ZAP Development Team
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.parosproxy.paros.db;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

import org.apache.log4j.Logger;

/**
* An abstract implementation of {@link Database}, it allows to manage {@link DatabaseListener DatabaseListeners}.
*
* @since TODO add version
*/
public abstract class AbstractDatabase implements Database {

protected final Logger logger = Logger.getLogger(getClass());

private final List<DatabaseListener> databaseListeners;

public AbstractDatabase() {
databaseListeners = new ArrayList<>();
}

/**
* Gets the database listeners added.
*
* @return the database listeners
* @see #addDatabaseListener(DatabaseListener)
*/
protected List<DatabaseListener> getDatabaseListeners() {
return databaseListeners;
}

@Override
public void addDatabaseListener(DatabaseListener listener) {
databaseListeners.add(listener);
}

@Override
public void removeDatabaseListener(DatabaseListener listener) {
databaseListeners.remove(listener);
}

@Override
public void close(boolean compact) {
close(compact, true);
}

/**
* {@inheritDoc}
* <p>
* Also, removes the database listeners added previously.
*/
@Override
public void close(boolean compact, boolean cleanup) {
removeDatabaseListeners();
}

/**
* Removes all database listeners added.
*
* @see #removeDatabaseListener(DatabaseListener)
*/
protected void removeDatabaseListeners() {
databaseListeners.clear();
}

/**
* Notifies the given listeners that the given database server was opened.
*
* @param listeners the listeners that will be notified
* @param databaseServer the database server that was opened
* @throws DatabaseException if an error occurred while notifying the database listeners.
*/
protected void notifyListenersDatabaseOpen(Collection<DatabaseListener> listeners, DatabaseServer databaseServer)
throws DatabaseException {
for (DatabaseListener databaseListener : listeners) {
try {
databaseListener.databaseOpen(databaseServer);
} catch (DatabaseUnsupportedException e) {
logger.error(e.getMessage(), e);
}
}
}

/**
* Notifies the database listeners added that the given database server was opened.
*
* @param databaseServer the database server that was opened
* @throws DatabaseException if an error occurred while notifying the database listeners.
*/
protected void notifyListenersDatabaseOpen(DatabaseServer databaseServer) throws DatabaseException {
notifyListenersDatabaseOpen(databaseListeners, databaseServer);
}
}
105 changes: 33 additions & 72 deletions src/org/parosproxy/paros/db/paros/ParosDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@
// ZAP: 2015/02/09 Issue 1525: Introduce a database interface layer to allow for alternative implementations
// ZAP: 2015/04/02 Issue 1582: Low memory option
// ZAP: 2016/02/10 Issue 1958: Allow to disable database (HSQLDB) log
// ZAP: 2016/04/22 Issue 2428: Memory leak on session creation/loading

package org.parosproxy.paros.db.paros;

import java.io.File;
import java.sql.SQLException;
import java.util.Vector;
import java.util.ArrayList;
import java.util.List;

import org.apache.log4j.Logger;
import org.parosproxy.paros.db.AbstractDatabase;
import org.parosproxy.paros.db.Database;
import org.parosproxy.paros.db.DatabaseException;
import org.parosproxy.paros.db.DatabaseListener;
import org.parosproxy.paros.db.DatabaseServer;
import org.parosproxy.paros.db.DatabaseUnsupportedException;
import org.parosproxy.paros.db.TableAlert;
import org.parosproxy.paros.db.TableContext;
import org.parosproxy.paros.db.TableHistory;
Expand All @@ -61,7 +61,7 @@



public class ParosDatabase implements Database {
public class ParosDatabase extends AbstractDatabase {

private ParosDatabaseServer databaseServer = null;
private TableHistory tableHistory = null;
Expand All @@ -76,11 +76,14 @@ public class ParosDatabase implements Database {
private TableParam tableParam = null;
private TableContext tableContext = null;
private TableStructure tableStructure = null;
// ZAP: Added Logger.
private static final Logger log = Logger.getLogger(ParosDatabase.class);

// ZAP: Added type arguments.
private Vector<DatabaseListener> listenerList = new Vector<>();
/**
* {@code DatabaseListener}s added internally when the {@code SqlDatabase} is constructed.
* <p>
* These listeners are kept during the lifetime of the database, while dynamically added listeners are removed once the
* database is closed.
*/
private List<DatabaseListener> internalDatabaseListeners = new ArrayList<>();

private DatabaseParam databaseOptions;

Expand All @@ -98,18 +101,15 @@ public ParosDatabase() {
tableContext = new ParosTableContext();
tableStructure = new ParosTableStructure();

addDatabaseListener(tableHistory);
addDatabaseListener(tableSession);
addDatabaseListener(tableAlert);
addDatabaseListener(tableScan);
// ZAP: Added statement.
addDatabaseListener(tableTag);
// ZAP: Added statement.
addDatabaseListener(tableSessionUrl);
// ZAP: Added statement.
addDatabaseListener(tableParam);
addDatabaseListener(tableContext);
addDatabaseListener(tableStructure);
internalDatabaseListeners.add(tableHistory);
internalDatabaseListeners.add(tableSession);
internalDatabaseListeners.add(tableAlert);
internalDatabaseListeners.add(tableScan);
internalDatabaseListeners.add(tableTag);
internalDatabaseListeners.add(tableSessionUrl);
internalDatabaseListeners.add(tableParam);
internalDatabaseListeners.add(tableContext);
internalDatabaseListeners.add(tableStructure);

}

Expand Down Expand Up @@ -144,74 +144,33 @@ public TableHistory getTableHistory() {
public TableSession getTableSession() {
return tableSession;
}

/* (non-Javadoc)
* @see org.parosproxy.paros.db.DatabaseIF#addDatabaseListener(org.parosproxy.paros.db.DatabaseListener)
*/
@Override
public void addDatabaseListener(DatabaseListener listener) {
listenerList.add(listener);

}

// ZAP: Changed parameter's type from SpiderListener to DatabaseListener.
/* (non-Javadoc)
* @see org.parosproxy.paros.db.DatabaseIF#removeDatabaseListener(org.parosproxy.paros.db.DatabaseListener)
*/
@Override
public void removeDatabaseListener(DatabaseListener listener) {
listenerList.remove(listener);
}

private void notifyListenerDatabaseOpen() throws DatabaseException {
DatabaseListener listener = null;

for (int i=0;i<listenerList.size();i++) {
// ZAP: Removed unnecessary cast.
listener = listenerList.get(i);
try {
listener.databaseOpen(getDatabaseServer());
} catch (DatabaseUnsupportedException e) {
log.error(e.getMessage(), e);
}
}
}

/* (non-Javadoc)
* @see org.parosproxy.paros.db.DatabaseIF#open(java.lang.String)
*/
@Override
public void open(String path) throws ClassNotFoundException, Exception {
// ZAP: Added log statement.
log.debug("open " + path);
logger.debug("open " + path);
setDatabaseServer(new ParosDatabaseServer(path, databaseOptions));
notifyListenerDatabaseOpen();

notifyListenersDatabaseOpen(internalDatabaseListeners, getDatabaseServer());
notifyListenersDatabaseOpen(getDatabaseServer());
}

/* (non-Javadoc)
* @see org.parosproxy.paros.db.DatabaseIF#close(boolean)
*/
// ZAP: Added JavaDoc.
@Override
public void close(boolean compact) {
// ZAP: Moved the content of this method to the method close(boolean,
// boolean) and changed to call that method instead.
close(compact, true);
}

/* (non-Javadoc)
* @see org.parosproxy.paros.db.DatabaseIF#deleteSession(java.lang.String)
*/
@Override
public void deleteSession(String sessionName) {
log.debug("deleteSession " + sessionName);
logger.debug("deleteSession " + sessionName);
if (databaseServer == null) {
return;
}
try {
databaseServer.shutdown(false);
} catch (SQLException e) {
log.error(e.getMessage(), e);
logger.error(e.getMessage(), e);
}

deleteDbFile(new File(sessionName));
Expand All @@ -225,10 +184,10 @@ public void deleteSession(String sessionName) {
}

private void deleteDbFile (File file) {
log.debug("Deleting " + file.getAbsolutePath());
logger.debug("Deleting " + file.getAbsolutePath());
if (file.exists()) {
if (! file.delete()) {
log.error("Failed to delete " + file.getAbsolutePath());
logger.error("Failed to delete " + file.getAbsolutePath());
}
}
}
Expand All @@ -241,8 +200,10 @@ private void deleteDbFile (File file) {
@Override
public void close(boolean compact, boolean cleanup) {
// ZAP: Added statement.
log.debug("close");
logger.debug("close");
if (databaseServer == null) return;

super.close(compact, cleanup);

try {
// ZAP: Added if block.
Expand All @@ -256,7 +217,7 @@ public void close(boolean compact, boolean cleanup) {
// ZAP: Changed to catch SQLException instead of Exception.
} catch (Exception e) {
// ZAP: Changed to log the exception.
log.error(e.getMessage(), e);
logger.error(e.getMessage(), e);
}
}

Expand Down
Loading

0 comments on commit 9c48c81

Please sign in to comment.