You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/10/08 13:13:48 UTC

[GitHub] [netbeans] sdedic commented on a change in pull request #2427: Maven branding configuration API and robustness improvement

sdedic commented on a change in pull request #2427:
URL: https://github.com/apache/netbeans/pull/2427#discussion_r501703529



##########
File path: java/maven/arch.xml
##########
@@ -169,6 +169,25 @@
           in case reparsing starts too soon (wasting CPU) or too late (impeding editing).
       </api>
   </p>
+  
+  <p>
+      <api category="devel" group="branding" name="DEFAULT_REUSE_OUTPUT" type="export">
+          Brand the <code>DEFAULT_REUSE_OUTPUT</code> key in a 
+          <code>org.netbeans.modules.maven.options.Bundle</code> file
+          with one of the values <code>true</code> or <code>false</code>
+          to specify the default behavior of reusing output by your application.
+          Use <code>never</code> value, if the reuse shall never be done,
+          regardless of the settings value.
+      </api> 
+      <api category="devel" group="branding" name="DEFAULT_COMPILE_ON_SAVE" type="export">
+          Brand the <code>DEFAULT_COMPILE_ON_SAVE</code> key in a 
+          <code>org.netbeans.modules.maven.api.execute.Bundle</code> file
+          with one of the values <code>all</code> or <code>node</code>

Review comment:
       Nitpick - typo: "node".

##########
File path: java/maven/src/org/netbeans/modules/maven/debug/MavenJPDAStart.java
##########
@@ -69,139 +72,126 @@
     private ClassPath additionalSourcePath;
     
     
-    private final Object[] lock = new Object[2];
-    
-    private Project project;
-    private final String actionName;
-    private final InputOutput io;
+    private final Project project;
+    private Future<?> lastFuture;
+
+    private MavenJPDAStart(Project p) {
+        this.project = p;
+    }
 
-    JPDAStart(InputOutput inputOutput, String actionName) {
-        io = inputOutput;
-        this.actionName = actionName;
+    /** Create and place into Project's Lookup.
+     * @param p the project to associate this start with
+     * @return new instance of the start infrastructure
+     */
+    public static MavenJPDAStart create(Project p) {
+        return new MavenJPDAStart(p);
     }
     
     /**
      * returns the port/address that the debugger listens to..
      */
-    public String execute(Project project) throws Throwable {
-        this.project = project;
-        io.getOut().println("JPDA Listening Start..."); //NOI18N
-//            getLog().debug("Entering synch lock"); //NOI18N
-        synchronized (lock) {
-//                getLog().debug("Entered synch lock"); //NOI18N
-            RP.post(this);
-//                    getLog().debug("Entering wait"); //NOI18N
-            lock.wait();
-//                    getLog().debug("Wait finished"); //NOI18N
-            if (lock[1] != null) {
-                throw ((Throwable) lock[1]); //NOI18N
+    public String execute(InputOutput io) throws Throwable {
+        Future<?> prev = lastFuture;
+        if (prev != null && !prev.isDone()) {
+            io.getOut().print("Cancelling previous JPDA listening..."); //NOI18N
+            if (prev.cancel(true)) {
+                io.getOut().println("done"); //NOI18N
+            } else {
+                io.getOut().println("failed"); //NOI18N
             }
         }
-        return (String)lock[0];
+        io.getOut().println("JPDA Listening Start..."); //NOI18N
+        Future<String> future = RP.submit(() -> {
+            return startDebugger(io);
+        });
+        lastFuture = future;

Review comment:
       May need some synchronization/flush when assigning to the shared field.

##########
File path: java/maven/src/org/netbeans/modules/maven/debug/MavenJPDAStart.java
##########
@@ -69,139 +72,126 @@
     private ClassPath additionalSourcePath;
     
     
-    private final Object[] lock = new Object[2];
-    
-    private Project project;
-    private final String actionName;
-    private final InputOutput io;
+    private final Project project;
+    private Future<?> lastFuture;
+
+    private MavenJPDAStart(Project p) {
+        this.project = p;
+    }
 
-    JPDAStart(InputOutput inputOutput, String actionName) {
-        io = inputOutput;
-        this.actionName = actionName;
+    /** Create and place into Project's Lookup.
+     * @param p the project to associate this start with
+     * @return new instance of the start infrastructure
+     */
+    public static MavenJPDAStart create(Project p) {
+        return new MavenJPDAStart(p);
     }
     
     /**
      * returns the port/address that the debugger listens to..
      */
-    public String execute(Project project) throws Throwable {
-        this.project = project;
-        io.getOut().println("JPDA Listening Start..."); //NOI18N
-//            getLog().debug("Entering synch lock"); //NOI18N
-        synchronized (lock) {
-//                getLog().debug("Entered synch lock"); //NOI18N
-            RP.post(this);
-//                    getLog().debug("Entering wait"); //NOI18N
-            lock.wait();
-//                    getLog().debug("Wait finished"); //NOI18N
-            if (lock[1] != null) {
-                throw ((Throwable) lock[1]); //NOI18N
+    public String execute(InputOutput io) throws Throwable {
+        Future<?> prev = lastFuture;
+        if (prev != null && !prev.isDone()) {
+            io.getOut().print("Cancelling previous JPDA listening..."); //NOI18N
+            if (prev.cancel(true)) {
+                io.getOut().println("done"); //NOI18N
+            } else {
+                io.getOut().println("failed"); //NOI18N
             }
         }
-        return (String)lock[0];
+        io.getOut().println("JPDA Listening Start..."); //NOI18N
+        Future<String> future = RP.submit(() -> {
+            return startDebugger(io);
+        });
+        lastFuture = future;
+        return future.get();
     }
     
-    @Override
-    public void run() {
-        synchronized (lock) {
-            
-            try {
-                
-                ListeningConnector lc = null;
-                Iterator i = Bootstrap.virtualMachineManager().
-                        listeningConnectors().iterator();
-                for (; i.hasNext();) {
-                    lc = (ListeningConnector) i.next();
-                    Transport t = lc.transport();
-                    if (t != null && t.name().equals(getTransport())) {
-                        break;
-                    }
-                }
-                if (lc == null) {
-                    throw new RuntimeException
-                            ("No trasports named " + getTransport() + " found!"); //NOI18N
-                }
-                // TODO: revisit later when http://developer.java.sun.com/developer/bugParade/bugs/4932074.html gets integrated into JDK
-                // This code parses the address string "HOST:PORT" to extract PORT and then point debugee to localhost:PORT
-                // This is NOT a clean solution to the problem but it SHOULD work in 99% cases
-                final Map args = lc.defaultArguments();
-                String address = lc.startListening(args);
-                try {
-                    int port = Integer.parseInt(address.substring(address.indexOf(':') + 1));
+    private String startDebugger(InputOutput io) throws IOException, IllegalConnectorArgumentsException {
+        String portOrAddress;
+        ListeningConnector lc = null;
+        Iterator i = Bootstrap.virtualMachineManager().
+                listeningConnectors().iterator();
+        for (; i.hasNext();) {
+            lc = (ListeningConnector) i.next();
+            Transport t = lc.transport();
+            if (t != null && t.name().equals(getTransport())) {
+                break;
+            }
+        }
+        if (lc == null) {
+            throw new RuntimeException
+                    ("No trasports named " + getTransport() + " found!"); //NOI18N
+        }
+        // TODO: revisit later when http://developer.java.sun.com/developer/bugParade/bugs/4932074.html gets integrated into JDK
+        // This code parses the address string "HOST:PORT" to extract PORT and then point debugee to localhost:PORT
+        // This is NOT a clean solution to the problem but it SHOULD work in 99% cases
+        final Map args = lc.defaultArguments();
+        String address = lc.startListening(args);
+        try {
+            int port = Integer.parseInt(address.substring(address.indexOf(':') + 1));
 //                    getProject ().setNewProperty (getAddressProperty (), "localhost:" + port);
-                    Connector.IntegerArgument portArg = (Connector.IntegerArgument) args.get("port"); //NOI18N
-                    portArg.setValue(port);
-                    lock[0] = Integer.toString(port);
-                } catch (NumberFormatException e) {
-                    // this address format is not known, use default
+            Connector.IntegerArgument portArg = (Connector.IntegerArgument) args.get("port"); //NOI18N
+            portArg.setValue(port);
+            portOrAddress = Integer.toString(port);
+        } catch (NumberFormatException e) {
+            // this address format is not known, use default
 //                    getProject ().setNewProperty (getAddressProperty (), address);
-                    lock[0] = address;
-                }
-                io.getOut().println("JPDA Address: " + address); //NOI18N
-                io.getOut().println("Port:" + lock[0]); //NOI18N
-                
-                ClassPath sourcePath = Utils.createSourcePath(project);
-                if (getAdditionalSourcePath() != null) {
-                    sourcePath = ClassPathSupport.createProxyClassPath(sourcePath, getAdditionalSourcePath());
-                }
-                ClassPath jdkSourcePath = Utils.createJDKSourcePath(project);
-                
-                if (getStopClassName() != null && getStopClassName().length() > 0) {
-                    final MethodBreakpoint b = getStopMethod() != null ? Utils.createBreakpoint(getStopClassName(), getStopMethod()) : Utils.createBreakpoint(getStopClassName());
-                    final Listener list = new Listener(b);
-                    b.addPropertyChangeListener(MethodBreakpoint.PROP_VALIDITY, new PropertyChangeListener() {
-                        @Override
-                        public void propertyChange(PropertyChangeEvent pce) {
-                            if (Breakpoint.VALIDITY.INVALID.equals(b.getValidity()) && getStopMethod() != null) {
-                                //when the original method with method is not available (maybe defined in parent class?), replace it with a class breakpoint
-                                DebuggerManager.getDebuggerManager().removeBreakpoint(b);
-                                MethodBreakpoint b2 = Utils.createBreakpoint(getStopClassName());
-                                list.replaceBreakpoint(b2);
-                            }
-                        }
-                    });
-                    DebuggerManager.getDebuggerManager().addDebuggerListener(
-                            DebuggerManager.PROP_DEBUGGER_ENGINES,
-                            list);
-                }
-                
-                final Map properties = new HashMap();
-                properties.put("sourcepath", sourcePath); //NOI18N
-                properties.put("name", getName()); //NOI18N
-                properties.put("jdksources", jdkSourcePath); //NOI18N
-                properties.put("baseDir", FileUtil.toFile(project.getProjectDirectory())); // NOI18N
-                if (RunUtils.isCompileOnSaveEnabled(project)) {
-                    properties.put ("listeningCP", "sourcepath"); // NOI18N
-                }
-                
-                final ListeningConnector flc = lc;
-                RP.post(new Runnable() {
+            portOrAddress = address;
+        }
+        io.getOut().println("JPDA Address: " + address); //NOI18N
+        io.getOut().println("Port:" + portOrAddress); //NOI18N
+
+        ClassPath sourcePath = Utils.createSourcePath(project);
+        if (getAdditionalSourcePath() != null) {
+            sourcePath = ClassPathSupport.createProxyClassPath(sourcePath, getAdditionalSourcePath());
+        }
+        ClassPath jdkSourcePath = Utils.createJDKSourcePath(project);
 
-                    @Override
-                    public void run() {
-                        try {
-                            JPDADebugger.startListening(flc, args,
-                                                        new Object[]{properties, project});
-                        }
-                        catch (DebuggerStartException ex) {
-                            io.getErr().println("Debugger Start Error."); //NOI18N
-                            Logger.getLogger(JPDAStart.class.getName()).log(Level.INFO, "Debugger Start Error.", ex);
-                        }
+        if (getStopClassName() != null && getStopClassName().length() > 0) {
+            final MethodBreakpoint b = getStopMethod() != null ? Utils.createBreakpoint(getStopClassName(), getStopMethod()) : Utils.createBreakpoint(getStopClassName());
+            final Listener list = new Listener(b);
+            b.addPropertyChangeListener(MethodBreakpoint.PROP_VALIDITY, new PropertyChangeListener() {
+                @Override
+                public void propertyChange(PropertyChangeEvent pce) {
+                    if (Breakpoint.VALIDITY.INVALID.equals(b.getValidity()) && getStopMethod() != null) {
+                        //when the original method with method is not available (maybe defined in parent class?), replace it with a class breakpoint
+                        DebuggerManager.getDebuggerManager().removeBreakpoint(b);
+                        MethodBreakpoint b2 = Utils.createBreakpoint(getStopClassName());
+                        list.replaceBreakpoint(b2);
                     }
-                });
-            } catch (java.io.IOException ioex) {
-                io.getErr().println("IO Error:"); //NOI18N
-//                org.openide.ErrorManager.getDefault().notify(ioex);
-                lock[1] = ioex;
-            } catch (com.sun.jdi.connect.IllegalConnectorArgumentsException icaex) {
-                io.getErr().println("Illegal Connector"); //NOI18N
-                lock[1] = icaex;
-            } finally {
-                lock.notify();
-            }
+                }
+            });
+            DebuggerManager.getDebuggerManager().addDebuggerListener(
+                    DebuggerManager.PROP_DEBUGGER_ENGINES,
+                    list);
         }
+
+        final Map properties = new HashMap();
+        properties.put("sourcepath", sourcePath); //NOI18N
+        properties.put("name", getName()); //NOI18N
+        properties.put("jdksources", jdkSourcePath); //NOI18N
+        properties.put("baseDir", FileUtil.toFile(project.getProjectDirectory())); // NOI18N
+        if (RunUtils.isCompileOnSaveEnabled(project)) {
+            properties.put ("listeningCP", "sourcepath"); // NOI18N
+        }
+
+        final ListeningConnector flc = lc;
         
+        lastFuture = RP.submit(() -> {

Review comment:
       Flush the `lastFuture` to other threads, next `execute` will run in other than RP.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists