#2556 extract breakpoint management to the common part

plugins/org.jkiss.dbeaver.debug.core/src/org/jkiss/dbeaver/debug/DBGBaseSession.java

@@ -3,6 +3,8 @@ package org.jkiss.dbeaver.debug;
 import java.sql.Connection;
 import java.sql.SQLException;
 import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -12,30 +14,32 @@ import org.jkiss.dbeaver.model.impl.jdbc.JDBCExecutionContext;
 import org.jkiss.dbeaver.model.runtime.VoidProgressMonitor;
 
 public abstract class DBGBaseSession implements DBGSession {
-    
+
     private static final Log log = Log.getLog(DBGBaseSession.class);
-    
+
     protected final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true);
 
     private final DBGBaseController controller;
 
-    protected FutureTask<DBGEvent> task;
+    private FutureTask<DBGEvent> task;
 
     private Thread workerThread = null;
 
     private JDBCExecutionContext connection = null;
 
+    private final List<DBGBreakpointDescriptor> breakpoints = new ArrayList<>(1);
+
     public DBGBaseSession(DBGBaseController controller) {
         this.controller = controller;
     }
-    
+
     /**
-     * Return connection used in debug session 
+     * Return connection used in debug session
      * 
      * @return java.sql.Connection
      * @throws DBGException
      */
-    //FIXME: rework to DBC API
+    // FIXME: rework to DBC API
     protected Connection getConnection() throws DBGException {
         try {
             return ((JDBCExecutionContext) connection).getConnection(new VoidProgressMonitor());
@@ -44,7 +48,7 @@ public abstract class DBGBaseSession implements DBGSession {
         }
     }
 
-    //FIXME: should be known during construction
+    // FIXME: should be known during construction
     protected void setConnection(JDBCExecutionContext connection) {
         this.connection = connection;
     }
@@ -52,9 +56,9 @@ public abstract class DBGBaseSession implements DBGSession {
     public DBGBaseController getController() {
         return controller;
     }
-    
+
     /**
-     * Return true if debug session up and running on server 
+     * Return true if debug session up and running on server
      * 
      * @return boolean
      */
@@ -72,80 +76,171 @@ public abstract class DBGBaseSession implements DBGSession {
     }
 
     /**
-     * Return true if session waiting target connection (on breakpoint, after step or continue) in debug thread
+     * Return true if session waiting target connection (on breakpoint, after
+     * step or continue) in debug thread
      * 
      * @return boolean
      */
-    public boolean isDone(){
-
-        if (task == null)
+    public boolean isDone() {
+        if (task == null) {
             return true;
-
+        }
         if (task.isDone()) {
             try {
                 DBGEvent dbgEvent = task.get();
                 getController().fireEvent(dbgEvent);
             } catch (InterruptedException e) {
-                log.error("DEBUG INTERRUPT ERROR ",e);
+                log.error("DEBUG INTERRUPT ERROR ", e);
                 return false;
             } catch (ExecutionException e) {
-                log.error("DEBUG WARNING ",e);
+                log.error("DEBUG WARNING ", e);
                 return false;
             }
             return true;
-
         }
-
         return false;
-
     }
-    
+
     /**
-     *  Start thread for SQL command
+     * Start thread for SQL command
      * 
      * @param commandSQL
      * @param name
      * @throws DBGException
      */
     protected void runAsync(String commandSQL, String name, DBGEvent event) throws DBGException {
-
         Connection connection = getConnection();
         try (Statement stmt = connection.createStatement()) {
-
             connection.setAutoCommit(false);
-
             DBGWorker worker = new DBGWorker(connection, commandSQL, event);
-
             task = new FutureTask<DBGEvent>(worker);
-
             workerThread = new Thread(task);
-
             workerThread.setName(name);
-
             workerThread.start();
-
         } catch (SQLException e) {
             throw new DBGException("SQL error", e);
-
         }
     }
 
     public void close() {
-
         lock.writeLock().lock();
-
         try {
+            if (!isDone()) {
+                task.cancel(true);
+            }
+            connection.close();
+        } finally {
+            lock.writeLock().unlock();
+        }
+    }
 
-                if (!isDone()) {
-                    task.cancel(true);
-                }
+    @Override
+    public void abort() throws DBGException {
+        acquireReadLock();
+        try (Statement stmt = getConnection().createStatement()) {
+            String sqlCommand = composeAbortCommand();
+            stmt.execute(sqlCommand);
+            // FIXME: move to finally?
+            task = null;
+        } catch (SQLException e) {
+            throw new DBGException("SQL error", e);
+        } finally {
+            lock.readLock().unlock();
+        }
+    }
+
+    protected abstract String composeAbortCommand();
 
-                connection.close();
+    @Override
+    public List<? extends DBGBreakpointDescriptor> getBreakpoints() {
+        return new ArrayList<DBGBreakpointDescriptor>(breakpoints);
+    }
 
+    @Override
+    public void addBreakpoint(DBGBreakpointDescriptor descriptor) throws DBGException {
+        acquireReadLock();
+        try {
+            try (Statement stmt = getConnection().createStatement()) {
+                String sqlQuery = composeAddBreakpointCommand(descriptor);
+                stmt.executeQuery(sqlQuery);
+            } catch (SQLException e) {
+                throw new DBGException("SQL error", e);
+            }
+            breakpoints.add(descriptor);
         } finally {
-            lock.writeLock().unlock();
+            lock.readLock().unlock();
+        }
+    }
+
+    protected abstract String composeAddBreakpointCommand(DBGBreakpointDescriptor descriptor);
+
+    @Override
+    public void removeBreakpoint(DBGBreakpointDescriptor bp) throws DBGException {
+        acquireReadLock();
+        try {
+            try (Statement stmt = getConnection().createStatement()) {
+                String sqlCommand = composeRemoveBreakpointCommand(bp);
+                stmt.executeQuery(sqlCommand);
+            } catch (SQLException e) {
+                throw new DBGException("SQL error", e);
+            }
+            breakpoints.remove(bp);
+        } finally {
+            lock.readLock().unlock();
         }
+    }
+
+    protected abstract String composeRemoveBreakpointCommand(DBGBreakpointDescriptor descriptor);
 
+    /**
+     * Try to acquire shared lock
+     * 
+     * @throws DBGException
+     */
+    protected void acquireReadLock() throws DBGException {
+        try {
+            lock.readLock().lockInterruptibly();
+        } catch (InterruptedException e1) {
+            throw new DBGException(e1.getMessage());
+        }
+        if (!isAttached()) {
+            lock.readLock().unlock();
+            throw new DBGException("Debug session not attached");
+        }
+        if (isWaiting()) {
+            lock.readLock().unlock();
+            throw new DBGException("Debug session in waiting state");
+        }
+        if (!isDone()) {
+            lock.readLock().unlock();
+            throw new DBGException("Debug session in incorrect state");
+        }
+
+    }
+
+    /**
+     * Try to acquire exclusive lock
+     * 
+     * @throws DBGException
+     */
+    protected void acquireWriteLock() throws DBGException {
+        try {
+            lock.writeLock().lockInterruptibly();
+        } catch (InterruptedException e1) {
+            throw new DBGException(e1.getMessage());
+        }
+        if (!isAttached()) {
+            lock.writeLock().unlock();
+            throw new DBGException("Debug session not attached");
+        }
+        if (isWaiting()) {
+            lock.writeLock().unlock();
+            throw new DBGException("Debug session in waiting state");
+        }
+        if (!isDone()) {
+            lock.writeLock().unlock();
+            throw new DBGException("Debug session in incorrect state");
+        }
     }
 
 }

plugins/org.jkiss.dbeaver.ext.postgresql.debug.core/src/org/jkiss/dbeaver/ext/postgresql/debug/internal/impl/PostgreDebugSession.java

@@ -81,8 +81,6 @@ public class PostgreDebugSession extends DBGBaseSession {
     private static final String SQL_DROP_BREAKPOINT = "select pldbg_drop_breakpoint(?sessionid, ?obj, ?line)";
     private static final String SQL_ATTACH_BREAKPOINT = "select pldbg_wait_for_breakpoint(?sessionid)";
 
-    private List<PostgreDebugBreakpointDescriptor> breakpoints = new ArrayList<PostgreDebugBreakpointDescriptor>(1);
-    
     /**
      * Create session with two description 
      * after creation session need to be attached to postgres procedure by attach method
@@ -158,65 +156,25 @@ public class PostgreDebugSession extends DBGBaseSession {
         return sessionInfo;
     }
 
-    @Override
-    public List<PostgreDebugBreakpointDescriptor> getBreakpoints() {
-        return breakpoints;
-    }
-
-    @Override
-    public void addBreakpoint(DBGBreakpointDescriptor descriptor) throws DBGException {
-
-        acquireReadLock();
-
-
-        try {
-            PostgreDebugBreakpointDescriptor bp = (PostgreDebugBreakpointDescriptor) descriptor;
-            PostgreDebugBreakpointProperties bpd = (PostgreDebugBreakpointProperties) bp.getProperties();
-            try (Statement stmt = getConnection().createStatement()) {
-
-                String sqlCommand = bpd.isGlobal() ? SQL_SET_GLOBAL_BREAKPOINT : SQL_SET_BREAKPOINT;
-
-                stmt.executeQuery(sqlCommand.replaceAll("\\?sessionid", String.valueOf(getSessionId()))
-                        .replaceAll("\\?obj", String.valueOf(descriptor.getObjectDescriptor().getID()))
-                        .replaceAll("\\?line", bpd.isOnStart() ? "-1" : String.valueOf(bpd.getLineNo()))
-                        .replaceAll("\\?target", bpd.isAll() ? "null"
-                                : String.valueOf(bpd.getTargetId())));
-
-            } catch (SQLException e) {
-                throw new DBGException("SQL error", e);
-            }
-            breakpoints.add(bp);
-
-        } finally {
-            lock.readLock().unlock();
-        }
-
+    protected String composeAddBreakpointCommand(DBGBreakpointDescriptor descriptor) {
+        PostgreDebugBreakpointDescriptor bp = (PostgreDebugBreakpointDescriptor) descriptor;
+        PostgreDebugBreakpointProperties bpd = (PostgreDebugBreakpointProperties) bp.getProperties();
+        String sqlPattern = bpd.isGlobal() ? SQL_SET_GLOBAL_BREAKPOINT : SQL_SET_BREAKPOINT;
+    
+        String sqlCommand = sqlPattern.replaceAll("\\?sessionid", String.valueOf(getSessionId()))
+                .replaceAll("\\?obj", String.valueOf(descriptor.getObjectDescriptor().getID()))
+                .replaceAll("\\?line", bpd.isOnStart() ? "-1" : String.valueOf(bpd.getLineNo()))
+                .replaceAll("\\?target", bpd.isAll() ? "null"
+                        : String.valueOf(bpd.getTargetId()));
+        return sqlCommand;
     }
 
-    @Override
-    public void removeBreakpoint(DBGBreakpointDescriptor bp) throws DBGException {
-
-        acquireReadLock();
-
-        try {
-
-            try (Statement stmt = getConnection().createStatement()) {
-
-                PostgreDebugBreakpointProperties properties = (PostgreDebugBreakpointProperties) bp.getProperties();
-                stmt.executeQuery(SQL_DROP_BREAKPOINT.replaceAll("\\?sessionid", String.valueOf(getSessionId()))
-                        .replaceAll("\\?obj", String.valueOf(bp.getObjectDescriptor().getID()))
-                        .replaceAll("\\?line", properties.isOnStart() ? "-1" : String.valueOf(properties.getLineNo())));
-
-            } catch (SQLException e) {
-                throw new DBGException("SQL error", e);
-            }
-
-            breakpoints.remove(bp);
-
-        } finally {
-            lock.readLock().unlock();
-        }
-
+    protected String composeRemoveBreakpointCommand(DBGBreakpointDescriptor bp) {
+        PostgreDebugBreakpointProperties properties = (PostgreDebugBreakpointProperties) bp.getProperties();
+        String sqlCommand = SQL_DROP_BREAKPOINT.replaceAll("\\?sessionid", String.valueOf(getSessionId()))
+                .replaceAll("\\?obj", String.valueOf(bp.getObjectDescriptor().getID()))
+                .replaceAll("\\?line", properties.isOnStart() ? "-1" : String.valueOf(properties.getLineNo()));
+        return sqlCommand;
     }
 
     @Override
@@ -260,23 +218,8 @@ public class PostgreDebugSession extends DBGBaseSession {
 
     }
 
-    @Override
-    public void abort() throws DBGException {
-
-        acquireReadLock();
-
-        try (Statement stmt = getConnection().createStatement()) {
-
-            stmt.execute(SQL_ABORT.replaceAll("\\?sessionid", String.valueOf(sessionId)));
-//FIXME: move to finally?
-            task = null;
-
-        } catch (SQLException e) {
-            throw new DBGException("SQL error", e);
-        } finally {
-            lock.readLock().unlock();
-        }
-
+    protected String composeAbortCommand() {
+        return SQL_ABORT.replaceAll("\\?sessionid", String.valueOf(sessionId));
     }
 
     @Override
@@ -380,7 +323,7 @@ public class PostgreDebugSession extends DBGBaseSession {
 
     @Override
     public String toString() {
-        return "PostgreDebugSession " + (isWaiting() ? "WAITING" : "READY") + " [sessionId=" + sessionId + ", breakpoints=" + breakpoints + "targetId=("
+        return "PostgreDebugSession " + (isWaiting() ? "WAITING" : "READY") + " [sessionId=" + sessionId + ", breakpoints=" + getBreakpoints() + "targetId=("
                 + targetId + ") Session=(" + sessionInfo.toString() + ") " + "]";
     }
 
@@ -398,70 +341,4 @@ public class PostgreDebugSession extends DBGBaseSession {
         return super.isAttached() && (sessionId > 0);
     }
 
-    /**
-     * Try to acquire shared lock 
-     * 
-     * @throws DBGException
-     */
-    private void acquireReadLock() throws DBGException {
-
-        try {
-
-            lock.readLock().lockInterruptibly();
-
-        } catch (InterruptedException e1) {
-
-            throw new DBGException(e1.getMessage());
-
-        }
-
-        if (!isAttached()) {
-            lock.readLock().unlock();
-            throw new DBGException("Debug session not attached");
-        }
-        
-        if (isWaiting()) {
-            lock.readLock().unlock();
-            throw new DBGException("Debug session in waiting state");
-        }
-
-        if (!isDone()) {
-            lock.readLock().unlock();
-            throw new DBGException("Debug session in incorrect state");
-        }
-
-    }
-
-    /**
-     *  Try to acquire exclusive lock 
-     * 
-     * @throws DBGException
-     */
-    private void acquireWriteLock() throws DBGException {
-
-        try {
-
-            lock.writeLock().lockInterruptibly();
-
-        } catch (InterruptedException e1) {
-            throw new DBGException(e1.getMessage());
-        }
-        
-        if (!isAttached()) {
-            lock.writeLock().unlock();
-            throw new DBGException("Debug session not attached");
-        }
-
-        if (isWaiting()) {
-            lock.writeLock().unlock();
-            throw new DBGException("Debug session in waiting state");
-        }
-
-        if (!isDone()) {
-            lock.writeLock().unlock();
-            throw new DBGException("Debug session in incorrect state");
-        }
-
-    }
-
 }