You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@karaf.apache.org by Guillaume Nodet <gn...@gmail.com> on 2012/02/16 10:53:26 UTC

Re: svn commit: r1132688 - in /karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console: completer/CommandsCompleter.java jline/Console.java

Could you please try to fix the changes you've made ?   You need to
call unget in a finally block in order to release the service
reference.
In case you wondered why it was coded that way, the getTarget and
ungetTarget methods did not exists at that time ... so I had to
fallback to reflection.

On Mon, Jun 6, 2011 at 18:05,  <cs...@apache.org> wrote:
> Author: cschneider
> Date: Mon Jun  6 16:05:00 2011
> New Revision: 1132688
>
> URL: http://svn.apache.org/viewvc?rev=1132688&view=rev
> Log:
> Refactorings in CommandsCompleter and Console
>
> Modified:
>    karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>    karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>
> Modified: karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
> URL: http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java?rev=1132688&r1=1132687&r2=1132688&view=diff
> ==============================================================================
> --- karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java (original)
> +++ karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java Mon Jun  6 16:05:00 2011
> @@ -18,7 +18,6 @@
>  */
>  package org.apache.karaf.shell.console.completer;
>
> -import java.lang.reflect.Field;
>  import java.util.ArrayList;
>  import java.util.Collections;
>  import java.util.HashSet;
> @@ -31,10 +30,7 @@ import org.apache.felix.gogo.runtime.Com
>  import org.apache.felix.gogo.runtime.CommandSessionImpl;
>  import org.apache.felix.service.command.CommandSession;
>  import org.apache.felix.service.command.Function;
> -import org.apache.karaf.shell.console.CompletableFunction;
>  import org.apache.karaf.shell.console.Completer;
> -import org.osgi.framework.BundleContext;
> -import org.osgi.framework.ServiceReference;
>
>  /**
>  * Like the {@link org.apache.karaf.shell.console.completer.CommandsCompleter} but does not use OSGi but is
> @@ -100,26 +96,17 @@ public class CommandsCompleter implement
>     }
>
>     protected Function unProxy(Function function) {
> -        try {
> -            if (function instanceof CommandProxy) {
> -                Field contextField = function.getClass().getDeclaredField("context");
> -                Field referenceField = function.getClass().getDeclaredField("reference");
> -                contextField.setAccessible(true);
> -                referenceField.setAccessible(true);
> -                BundleContext context = (BundleContext) contextField.get(function);
> -                ServiceReference reference = (ServiceReference) referenceField.get(function);
> -                Object target = context.getService(reference);
> -                try {
> -                    if (target instanceof Function) {
> -                        function = (Function) target;
> -                    }
> -                } finally {
> -                    context.ungetService(reference);
> -                }
> -            }
> -        } catch (Throwable t) {
> -        }
> -        return function;
> +       if (function instanceof CommandProxy) {
> +               CommandProxy proxy = (CommandProxy) function;
> +               Object target = proxy.getTarget();
> +                       if (target instanceof Function) {
> +                               return (Function) target;
> +                       } else {
> +                               return function;
> +                       }
> +       } else {
> +               return function;
> +       }
>     }
>
>  }
>
> Modified: karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
> URL: http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java?rev=1132688&r1=1132687&r2=1132688&view=diff
> ==============================================================================
> --- karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java (original)
> +++ karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java Mon Jun  6 16:05:00 2011
> @@ -182,57 +182,10 @@ public class Console implements Runnable
>         welcome();
>         setSessionProperties();
>         String scriptFileName = System.getProperty(SHELL_INIT_SCRIPT);
> -        if (scriptFileName != null) {
> -            Reader r = null;
> -            try {
> -                File scriptFile = new File(scriptFileName);
> -                r = new InputStreamReader(new FileInputStream(scriptFile));
> -                CharArrayWriter w = new CharArrayWriter();
> -                int n;
> -                char[] buf = new char[8192];
> -                while ((n = r.read(buf)) > 0) {
> -                    w.write(buf, 0, n);
> -                }
> -                session.execute(new String(w.toCharArray()));
> -            } catch (Exception e) {
> -                LOGGER.debug("Error in initialization script", e);
> -                System.err.println("Error in initialization script: " + e.getMessage());
> -            } finally {
> -                if (r != null) {
> -                    try {
> -                        r.close();
> -                    } catch (IOException e) {
> -                        // Ignore
> -                    }
> -                }
> -            }
> -        }
> +        executeScript(scriptFileName);
>         while (running) {
>             try {
> -                String command = null;
> -                boolean loop = true;
> -                boolean first = true;
> -                while (loop) {
> -                    checkInterrupt();
> -                    String line = reader.readLine(first ? getPrompt() : "> ");
> -                    if (line == null)
> -                    {
> -                        break;
> -                    }
> -                    if (command == null) {
> -                        command = line;
> -                    } else {
> -                        command += " " + line;
> -                    }
> -                    reader.getHistory().replace(command);
> -                    try {
> -                        new Parser(command).program();
> -                        loop = false;
> -                    } catch (Exception e) {
> -                        loop = true;
> -                        first = false;
> -                    }
> -                }
> +                String command = readAndParseCommand();
>                 if (command == null) {
>                     break;
>                 }
> @@ -254,35 +207,7 @@ public class Console implements Runnable
>             }
>             catch (Throwable t)
>             {
> -                try {
> -                    LOGGER.info("Exception caught while executing command", t);
> -                    session.put(LAST_EXCEPTION, t);
> -                    if (t instanceof CommandException) {
> -                        session.getConsole().println(((CommandException) t).getNiceHelp());
> -                    } else if (t instanceof CommandNotFoundException) {
> -                        String str = Ansi.ansi()
> -                            .fg(Ansi.Color.RED)
> -                            .a("Command not found: ")
> -                            .a(Ansi.Attribute.INTENSITY_BOLD)
> -                            .a(((CommandNotFoundException) t).getCommand())
> -                            .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
> -                            .fg(Ansi.Color.DEFAULT).toString();
> -                        session.getConsole().println(str);
> -                    }
> -                    if ( getBoolean(PRINT_STACK_TRACES)) {
> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
> -                        t.printStackTrace(session.getConsole());
> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
> -                    }
> -                    else if (!(t instanceof CommandException) && !(t instanceof CommandNotFoundException)) {
> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
> -                        session.getConsole().println("Error executing command: "
> -                                + (t.getMessage() != null ? t.getMessage() : t.getClass().getName()));
> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
> -                    }
> -                } catch (Exception ignore) {
> -                        // ignore
> -                }
> +                logException(t);
>             }
>         }
>         close();
> @@ -293,6 +218,94 @@ public class Console implements Runnable
>         }
>     }
>
> +       private void logException(Throwable t) {
> +               try {
> +                   LOGGER.info("Exception caught while executing command", t);
> +                   session.put(LAST_EXCEPTION, t);
> +                   if (t instanceof CommandException) {
> +                       session.getConsole().println(((CommandException) t).getNiceHelp());
> +                   } else if (t instanceof CommandNotFoundException) {
> +                       String str = Ansi.ansi()
> +                           .fg(Ansi.Color.RED)
> +                           .a("Command not found: ")
> +                           .a(Ansi.Attribute.INTENSITY_BOLD)
> +                           .a(((CommandNotFoundException) t).getCommand())
> +                           .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
> +                           .fg(Ansi.Color.DEFAULT).toString();
> +                       session.getConsole().println(str);
> +                   }
> +                   if ( getBoolean(PRINT_STACK_TRACES)) {
> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
> +                       t.printStackTrace(session.getConsole());
> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
> +                   }
> +                   else if (!(t instanceof CommandException) && !(t instanceof CommandNotFoundException)) {
> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
> +                       session.getConsole().println("Error executing command: "
> +                               + (t.getMessage() != null ? t.getMessage() : t.getClass().getName()));
> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
> +                   }
> +               } catch (Exception ignore) {
> +                       // ignore
> +               }
> +       }
> +
> +       private String readAndParseCommand() throws IOException {
> +               String command = null;
> +               boolean loop = true;
> +               boolean first = true;
> +               while (loop) {
> +                   checkInterrupt();
> +                   String line = reader.readLine(first ? getPrompt() : "> ");
> +                   if (line == null)
> +                   {
> +                       break;
> +                   }
> +                   if (command == null) {
> +                       command = line;
> +                   } else {
> +                       command += " " + line;
> +                   }
> +                   reader.getHistory().replace(command);
> +                   try {
> +                       new Parser(command).program();
> +                       loop = false;
> +                   } catch (Exception e) {
> +                       loop = true;
> +                       first = false;
> +                   }
> +               }
> +               return command;
> +       }
> +
> +       private void executeScript(String scriptFileName) {
> +               if (scriptFileName != null) {
> +            Reader r = null;
> +            try {
> +                File scriptFile = new File(scriptFileName);
> +                r = new InputStreamReader(new FileInputStream(scriptFile));
> +                CharArrayWriter w = new CharArrayWriter();
> +                int n;
> +                char[] buf = new char[8192];
> +                while ((n = r.read(buf)) > 0) {
> +                    w.write(buf, 0, n);
> +                }
> +                session.execute(new String(w.toCharArray()));
> +            } catch (Exception e) {
> +                LOGGER.debug("Error in initialization script", e);
> +                System.err.println("Error in initialization script: " + e.getMessage());
> +            } finally {
> +                if (r != null) {
> +                    try {
> +                        r.close();
> +                    } catch (IOException e) {
> +                        // Ignore
> +                    }
> +                }
> +            }
> +        }
> +       }
> +
>     protected boolean getBoolean(String name) {
>         Object s = session.get(name);
>         if (s == null) {
>
>



-- 
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com

Re: svn commit: r1132688 - in /karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console: completer/CommandsCompleter.java jline/Console.java

Posted by Christian Schneider <ch...@die-schneider.net>.
I now added the ungetTarget add about the same position where it was in 
the orig code. I am not sure though if that is correct.
The problem is that the service is still used after the unget. I would 
have thought that unget should be used only when the service is not 
needed anymore.

Christian


Am 17.02.2012 17:31, schrieb Guillaume Nodet:
> Not sure to understand what you mean. When you call getTarget, the
> underlying code [1] calls context.getService
> so you need to call ungetTarget to release this reference.
>
> [1] https://github.com/apache/felix/blob/trunk/gogo/runtime/src/main/java/org/apache/felix/gogo/runtime/CommandProxy.java#L48
>
> On Fri, Feb 17, 2012 at 12:36, Christian Schneider
> <ch...@die-schneider.net>  wrote:
>> Hi Guillaume,
>>
>> I just checked the class you mentioned. It does not seem to contain
>> context.getService anymore. So I think the code already looks good now.
>>
>> Christian
>>
>> Am 16.02.2012 10:53, schrieb Guillaume Nodet:
>>> Could you please try to fix the changes you've made ?   You need to
>>> call unget in a finally block in order to release the service
>>> reference.
>>> In case you wondered why it was coded that way, the getTarget and
>>> ungetTarget methods did not exists at that time ... so I had to
>>> fallback to reflection.
>>>
>>> On Mon, Jun 6, 2011 at 18:05,<cs...@apache.org>    wrote:
>>>> Author: cschneider
>>>> Date: Mon Jun  6 16:05:00 2011
>>>> New Revision: 1132688
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1132688&view=rev
>>>> Log:
>>>> Refactorings in CommandsCompleter and Console
>>>>
>>>> Modified:
>>>>
>>>>   karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>>
>>>>   karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>>
>>>> Modified:
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>> (original)
>>>> +++
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>> Mon Jun  6 16:05:00 2011
>>>> @@ -18,7 +18,6 @@
>>>>   */
>>>>   package org.apache.karaf.shell.console.completer;
>>>>
>>>> -import java.lang.reflect.Field;
>>>>   import java.util.ArrayList;
>>>>   import java.util.Collections;
>>>>   import java.util.HashSet;
>>>> @@ -31,10 +30,7 @@ import org.apache.felix.gogo.runtime.Com
>>>>   import org.apache.felix.gogo.runtime.CommandSessionImpl;
>>>>   import org.apache.felix.service.command.CommandSession;
>>>>   import org.apache.felix.service.command.Function;
>>>> -import org.apache.karaf.shell.console.CompletableFunction;
>>>>   import org.apache.karaf.shell.console.Completer;
>>>> -import org.osgi.framework.BundleContext;
>>>> -import org.osgi.framework.ServiceReference;
>>>>
>>>>   /**
>>>>   * Like the {@link
>>>> org.apache.karaf.shell.console.completer.CommandsCompleter} but does not use
>>>> OSGi but is
>>>> @@ -100,26 +96,17 @@ public class CommandsCompleter implement
>>>>      }
>>>>
>>>>      protected Function unProxy(Function function) {
>>>> -        try {
>>>> -            if (function instanceof CommandProxy) {
>>>> -                Field contextField =
>>>> function.getClass().getDeclaredField("context");
>>>> -                Field referenceField =
>>>> function.getClass().getDeclaredField("reference");
>>>> -                contextField.setAccessible(true);
>>>> -                referenceField.setAccessible(true);
>>>> -                BundleContext context = (BundleContext)
>>>> contextField.get(function);
>>>> -                ServiceReference reference = (ServiceReference)
>>>> referenceField.get(function);
>>>> -                Object target = context.getService(reference);
>>>> -                try {
>>>> -                    if (target instanceof Function) {
>>>> -                        function = (Function) target;
>>>> -                    }
>>>> -                } finally {
>>>> -                    context.ungetService(reference);
>>>> -                }
>>>> -            }
>>>> -        } catch (Throwable t) {
>>>> -        }
>>>> -        return function;
>>>> +       if (function instanceof CommandProxy) {
>>>> +               CommandProxy proxy = (CommandProxy) function;
>>>> +               Object target = proxy.getTarget();
>>>> +                       if (target instanceof Function) {
>>>> +                               return (Function) target;
>>>> +                       } else {
>>>> +                               return function;
>>>> +                       }
>>>> +       } else {
>>>> +               return function;
>>>> +       }
>>>>      }
>>>>
>>>>   }
>>>>
>>>> Modified:
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>> (original)
>>>> +++
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>> Mon Jun  6 16:05:00 2011
>>>> @@ -182,57 +182,10 @@ public class Console implements Runnable
>>>>          welcome();
>>>>          setSessionProperties();
>>>>          String scriptFileName = System.getProperty(SHELL_INIT_SCRIPT);
>>>> -        if (scriptFileName != null) {
>>>> -            Reader r = null;
>>>> -            try {
>>>> -                File scriptFile = new File(scriptFileName);
>>>> -                r = new InputStreamReader(new
>>>> FileInputStream(scriptFile));
>>>> -                CharArrayWriter w = new CharArrayWriter();
>>>> -                int n;
>>>> -                char[] buf = new char[8192];
>>>> -                while ((n = r.read(buf))>    0) {
>>>> -                    w.write(buf, 0, n);
>>>> -                }
>>>> -                session.execute(new String(w.toCharArray()));
>>>> -            } catch (Exception e) {
>>>> -                LOGGER.debug("Error in initialization script", e);
>>>> -                System.err.println("Error in initialization script: " +
>>>> e.getMessage());
>>>> -            } finally {
>>>> -                if (r != null) {
>>>> -                    try {
>>>> -                        r.close();
>>>> -                    } catch (IOException e) {
>>>> -                        // Ignore
>>>> -                    }
>>>> -                }
>>>> -            }
>>>> -        }
>>>> +        executeScript(scriptFileName);
>>>>          while (running) {
>>>>              try {
>>>> -                String command = null;
>>>> -                boolean loop = true;
>>>> -                boolean first = true;
>>>> -                while (loop) {
>>>> -                    checkInterrupt();
>>>> -                    String line = reader.readLine(first ? getPrompt() :
>>>> ">    ");
>>>> -                    if (line == null)
>>>> -                    {
>>>> -                        break;
>>>> -                    }
>>>> -                    if (command == null) {
>>>> -                        command = line;
>>>> -                    } else {
>>>> -                        command += " " + line;
>>>> -                    }
>>>> -                    reader.getHistory().replace(command);
>>>> -                    try {
>>>> -                        new Parser(command).program();
>>>> -                        loop = false;
>>>> -                    } catch (Exception e) {
>>>> -                        loop = true;
>>>> -                        first = false;
>>>> -                    }
>>>> -                }
>>>> +                String command = readAndParseCommand();
>>>>                  if (command == null) {
>>>>                      break;
>>>>                  }
>>>> @@ -254,35 +207,7 @@ public class Console implements Runnable
>>>>              }
>>>>              catch (Throwable t)
>>>>              {
>>>> -                try {
>>>> -                    LOGGER.info("Exception caught while executing
>>>> command", t);
>>>> -                    session.put(LAST_EXCEPTION, t);
>>>> -                    if (t instanceof CommandException) {
>>>> -                        session.getConsole().println(((CommandException)
>>>> t).getNiceHelp());
>>>> -                    } else if (t instanceof CommandNotFoundException) {
>>>> -                        String str = Ansi.ansi()
>>>> -                            .fg(Ansi.Color.RED)
>>>> -                            .a("Command not found: ")
>>>> -                            .a(Ansi.Attribute.INTENSITY_BOLD)
>>>> -                            .a(((CommandNotFoundException)
>>>> t).getCommand())
>>>> -                            .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>>>> -                            .fg(Ansi.Color.DEFAULT).toString();
>>>> -                        session.getConsole().println(str);
>>>> -                    }
>>>> -                    if ( getBoolean(PRINT_STACK_TRACES)) {
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> -                        t.printStackTrace(session.getConsole());
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> -                    }
>>>> -                    else if (!(t instanceof CommandException)&&    !(t
>>>> instanceof CommandNotFoundException)) {
>>>>
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> -                        session.getConsole().println("Error executing
>>>> command: "
>>>> -                                + (t.getMessage() != null ?
>>>> t.getMessage() : t.getClass().getName()));
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> -                    }
>>>> -                } catch (Exception ignore) {
>>>> -                        // ignore
>>>> -                }
>>>> +                logException(t);
>>>>              }
>>>>          }
>>>>          close();
>>>> @@ -293,6 +218,94 @@ public class Console implements Runnable
>>>>          }
>>>>      }
>>>>
>>>> +       private void logException(Throwable t) {
>>>> +               try {
>>>> +                   LOGGER.info("Exception caught while executing
>>>> command", t);
>>>> +                   session.put(LAST_EXCEPTION, t);
>>>> +                   if (t instanceof CommandException) {
>>>> +                       session.getConsole().println(((CommandException)
>>>> t).getNiceHelp());
>>>> +                   } else if (t instanceof CommandNotFoundException) {
>>>> +                       String str = Ansi.ansi()
>>>> +                           .fg(Ansi.Color.RED)
>>>> +                           .a("Command not found: ")
>>>> +                           .a(Ansi.Attribute.INTENSITY_BOLD)
>>>> +                           .a(((CommandNotFoundException)
>>>> t).getCommand())
>>>> +                           .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>>>> +                           .fg(Ansi.Color.DEFAULT).toString();
>>>> +                       session.getConsole().println(str);
>>>> +                   }
>>>> +                   if ( getBoolean(PRINT_STACK_TRACES)) {
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> +                       t.printStackTrace(session.getConsole());
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> +                   }
>>>> +                   else if (!(t instanceof CommandException)&&    !(t
>>>> instanceof CommandNotFoundException)) {
>>>>
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> +                       session.getConsole().println("Error executing
>>>> command: "
>>>> +                               + (t.getMessage() != null ?
>>>> t.getMessage() : t.getClass().getName()));
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> +                   }
>>>> +               } catch (Exception ignore) {
>>>> +                       // ignore
>>>> +               }
>>>> +       }
>>>> +
>>>> +       private String readAndParseCommand() throws IOException {
>>>> +               String command = null;
>>>> +               boolean loop = true;
>>>> +               boolean first = true;
>>>> +               while (loop) {
>>>> +                   checkInterrupt();
>>>> +                   String line = reader.readLine(first ? getPrompt() :
>>>> ">    ");
>>>> +                   if (line == null)
>>>> +                   {
>>>> +                       break;
>>>> +                   }
>>>> +                   if (command == null) {
>>>> +                       command = line;
>>>> +                   } else {
>>>> +                       command += " " + line;
>>>> +                   }
>>>> +                   reader.getHistory().replace(command);
>>>> +                   try {
>>>> +                       new Parser(command).program();
>>>> +                       loop = false;
>>>> +                   } catch (Exception e) {
>>>> +                       loop = true;
>>>> +                       first = false;
>>>> +                   }
>>>> +               }
>>>> +               return command;
>>>> +       }
>>>> +
>>>> +       private void executeScript(String scriptFileName) {
>>>> +               if (scriptFileName != null) {
>>>> +            Reader r = null;
>>>> +            try {
>>>> +                File scriptFile = new File(scriptFileName);
>>>> +                r = new InputStreamReader(new
>>>> FileInputStream(scriptFile));
>>>> +                CharArrayWriter w = new CharArrayWriter();
>>>> +                int n;
>>>> +                char[] buf = new char[8192];
>>>> +                while ((n = r.read(buf))>    0) {
>>>> +                    w.write(buf, 0, n);
>>>> +                }
>>>> +                session.execute(new String(w.toCharArray()));
>>>> +            } catch (Exception e) {
>>>> +                LOGGER.debug("Error in initialization script", e);
>>>> +                System.err.println("Error in initialization script: " +
>>>> e.getMessage());
>>>> +            } finally {
>>>> +                if (r != null) {
>>>> +                    try {
>>>> +                        r.close();
>>>> +                    } catch (IOException e) {
>>>> +                        // Ignore
>>>> +                    }
>>>> +                }
>>>> +            }
>>>> +        }
>>>> +       }
>>>> +
>>>>      protected boolean getBoolean(String name) {
>>>>          Object s = session.get(name);
>>>>          if (s == null) {
>>>>
>>>>
>>>
>>
>> --
>>
>> Christian Schneider
>> http://www.liquid-reality.de
>>
>> Open Source Architect
>> Talend Application Integration Division http://www.talend.com
>>
>
>


-- 
Christian Schneider
http://www.liquid-reality.de

Open Source Architect
Talend Application Integration Division http://www.talend.com


Re: svn commit: r1132688 - in /karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console: completer/CommandsCompleter.java jline/Console.java

Posted by Christian Schneider <ch...@die-schneider.net>.
Ah there the getService went. I did not find that. We should document 
that as this is really not obvious. getTarget sounds like simply
following a pointer .. I will take care of doing both.

Christian


Am 17.02.2012 17:31, schrieb Guillaume Nodet:
> Not sure to understand what you mean. When you call getTarget, the
> underlying code [1] calls context.getService
> so you need to call ungetTarget to release this reference.
>
> [1] https://github.com/apache/felix/blob/trunk/gogo/runtime/src/main/java/org/apache/felix/gogo/runtime/CommandProxy.java#L48
>
> On Fri, Feb 17, 2012 at 12:36, Christian Schneider
> <ch...@die-schneider.net>  wrote:
>> Hi Guillaume,
>>
>> I just checked the class you mentioned. It does not seem to contain
>> context.getService anymore. So I think the code already looks good now.
>>
>> Christian
>>
>> Am 16.02.2012 10:53, schrieb Guillaume Nodet:
>>> Could you please try to fix the changes you've made ?   You need to
>>> call unget in a finally block in order to release the service
>>> reference.
>>> In case you wondered why it was coded that way, the getTarget and
>>> ungetTarget methods did not exists at that time ... so I had to
>>> fallback to reflection.
>>>
>>> On Mon, Jun 6, 2011 at 18:05,<cs...@apache.org>    wrote:
>>>> Author: cschneider
>>>> Date: Mon Jun  6 16:05:00 2011
>>>> New Revision: 1132688
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1132688&view=rev
>>>> Log:
>>>> Refactorings in CommandsCompleter and Console
>>>>
>>>> Modified:
>>>>
>>>>   karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>>
>>>>   karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>>
>>>> Modified:
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>> (original)
>>>> +++
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>> Mon Jun  6 16:05:00 2011
>>>> @@ -18,7 +18,6 @@
>>>>   */
>>>>   package org.apache.karaf.shell.console.completer;
>>>>
>>>> -import java.lang.reflect.Field;
>>>>   import java.util.ArrayList;
>>>>   import java.util.Collections;
>>>>   import java.util.HashSet;
>>>> @@ -31,10 +30,7 @@ import org.apache.felix.gogo.runtime.Com
>>>>   import org.apache.felix.gogo.runtime.CommandSessionImpl;
>>>>   import org.apache.felix.service.command.CommandSession;
>>>>   import org.apache.felix.service.command.Function;
>>>> -import org.apache.karaf.shell.console.CompletableFunction;
>>>>   import org.apache.karaf.shell.console.Completer;
>>>> -import org.osgi.framework.BundleContext;
>>>> -import org.osgi.framework.ServiceReference;
>>>>
>>>>   /**
>>>>   * Like the {@link
>>>> org.apache.karaf.shell.console.completer.CommandsCompleter} but does not use
>>>> OSGi but is
>>>> @@ -100,26 +96,17 @@ public class CommandsCompleter implement
>>>>      }
>>>>
>>>>      protected Function unProxy(Function function) {
>>>> -        try {
>>>> -            if (function instanceof CommandProxy) {
>>>> -                Field contextField =
>>>> function.getClass().getDeclaredField("context");
>>>> -                Field referenceField =
>>>> function.getClass().getDeclaredField("reference");
>>>> -                contextField.setAccessible(true);
>>>> -                referenceField.setAccessible(true);
>>>> -                BundleContext context = (BundleContext)
>>>> contextField.get(function);
>>>> -                ServiceReference reference = (ServiceReference)
>>>> referenceField.get(function);
>>>> -                Object target = context.getService(reference);
>>>> -                try {
>>>> -                    if (target instanceof Function) {
>>>> -                        function = (Function) target;
>>>> -                    }
>>>> -                } finally {
>>>> -                    context.ungetService(reference);
>>>> -                }
>>>> -            }
>>>> -        } catch (Throwable t) {
>>>> -        }
>>>> -        return function;
>>>> +       if (function instanceof CommandProxy) {
>>>> +               CommandProxy proxy = (CommandProxy) function;
>>>> +               Object target = proxy.getTarget();
>>>> +                       if (target instanceof Function) {
>>>> +                               return (Function) target;
>>>> +                       } else {
>>>> +                               return function;
>>>> +                       }
>>>> +       } else {
>>>> +               return function;
>>>> +       }
>>>>      }
>>>>
>>>>   }
>>>>
>>>> Modified:
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>> (original)
>>>> +++
>>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>> Mon Jun  6 16:05:00 2011
>>>> @@ -182,57 +182,10 @@ public class Console implements Runnable
>>>>          welcome();
>>>>          setSessionProperties();
>>>>          String scriptFileName = System.getProperty(SHELL_INIT_SCRIPT);
>>>> -        if (scriptFileName != null) {
>>>> -            Reader r = null;
>>>> -            try {
>>>> -                File scriptFile = new File(scriptFileName);
>>>> -                r = new InputStreamReader(new
>>>> FileInputStream(scriptFile));
>>>> -                CharArrayWriter w = new CharArrayWriter();
>>>> -                int n;
>>>> -                char[] buf = new char[8192];
>>>> -                while ((n = r.read(buf))>    0) {
>>>> -                    w.write(buf, 0, n);
>>>> -                }
>>>> -                session.execute(new String(w.toCharArray()));
>>>> -            } catch (Exception e) {
>>>> -                LOGGER.debug("Error in initialization script", e);
>>>> -                System.err.println("Error in initialization script: " +
>>>> e.getMessage());
>>>> -            } finally {
>>>> -                if (r != null) {
>>>> -                    try {
>>>> -                        r.close();
>>>> -                    } catch (IOException e) {
>>>> -                        // Ignore
>>>> -                    }
>>>> -                }
>>>> -            }
>>>> -        }
>>>> +        executeScript(scriptFileName);
>>>>          while (running) {
>>>>              try {
>>>> -                String command = null;
>>>> -                boolean loop = true;
>>>> -                boolean first = true;
>>>> -                while (loop) {
>>>> -                    checkInterrupt();
>>>> -                    String line = reader.readLine(first ? getPrompt() :
>>>> ">    ");
>>>> -                    if (line == null)
>>>> -                    {
>>>> -                        break;
>>>> -                    }
>>>> -                    if (command == null) {
>>>> -                        command = line;
>>>> -                    } else {
>>>> -                        command += " " + line;
>>>> -                    }
>>>> -                    reader.getHistory().replace(command);
>>>> -                    try {
>>>> -                        new Parser(command).program();
>>>> -                        loop = false;
>>>> -                    } catch (Exception e) {
>>>> -                        loop = true;
>>>> -                        first = false;
>>>> -                    }
>>>> -                }
>>>> +                String command = readAndParseCommand();
>>>>                  if (command == null) {
>>>>                      break;
>>>>                  }
>>>> @@ -254,35 +207,7 @@ public class Console implements Runnable
>>>>              }
>>>>              catch (Throwable t)
>>>>              {
>>>> -                try {
>>>> -                    LOGGER.info("Exception caught while executing
>>>> command", t);
>>>> -                    session.put(LAST_EXCEPTION, t);
>>>> -                    if (t instanceof CommandException) {
>>>> -                        session.getConsole().println(((CommandException)
>>>> t).getNiceHelp());
>>>> -                    } else if (t instanceof CommandNotFoundException) {
>>>> -                        String str = Ansi.ansi()
>>>> -                            .fg(Ansi.Color.RED)
>>>> -                            .a("Command not found: ")
>>>> -                            .a(Ansi.Attribute.INTENSITY_BOLD)
>>>> -                            .a(((CommandNotFoundException)
>>>> t).getCommand())
>>>> -                            .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>>>> -                            .fg(Ansi.Color.DEFAULT).toString();
>>>> -                        session.getConsole().println(str);
>>>> -                    }
>>>> -                    if ( getBoolean(PRINT_STACK_TRACES)) {
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> -                        t.printStackTrace(session.getConsole());
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> -                    }
>>>> -                    else if (!(t instanceof CommandException)&&    !(t
>>>> instanceof CommandNotFoundException)) {
>>>>
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> -                        session.getConsole().println("Error executing
>>>> command: "
>>>> -                                + (t.getMessage() != null ?
>>>> t.getMessage() : t.getClass().getName()));
>>>> -
>>>>   session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> -                    }
>>>> -                } catch (Exception ignore) {
>>>> -                        // ignore
>>>> -                }
>>>> +                logException(t);
>>>>              }
>>>>          }
>>>>          close();
>>>> @@ -293,6 +218,94 @@ public class Console implements Runnable
>>>>          }
>>>>      }
>>>>
>>>> +       private void logException(Throwable t) {
>>>> +               try {
>>>> +                   LOGGER.info("Exception caught while executing
>>>> command", t);
>>>> +                   session.put(LAST_EXCEPTION, t);
>>>> +                   if (t instanceof CommandException) {
>>>> +                       session.getConsole().println(((CommandException)
>>>> t).getNiceHelp());
>>>> +                   } else if (t instanceof CommandNotFoundException) {
>>>> +                       String str = Ansi.ansi()
>>>> +                           .fg(Ansi.Color.RED)
>>>> +                           .a("Command not found: ")
>>>> +                           .a(Ansi.Attribute.INTENSITY_BOLD)
>>>> +                           .a(((CommandNotFoundException)
>>>> t).getCommand())
>>>> +                           .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>>>> +                           .fg(Ansi.Color.DEFAULT).toString();
>>>> +                       session.getConsole().println(str);
>>>> +                   }
>>>> +                   if ( getBoolean(PRINT_STACK_TRACES)) {
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> +                       t.printStackTrace(session.getConsole());
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> +                   }
>>>> +                   else if (!(t instanceof CommandException)&&    !(t
>>>> instanceof CommandNotFoundException)) {
>>>>
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>>> +                       session.getConsole().println("Error executing
>>>> command: "
>>>> +                               + (t.getMessage() != null ?
>>>> t.getMessage() : t.getClass().getName()));
>>>> +
>>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>>> +                   }
>>>> +               } catch (Exception ignore) {
>>>> +                       // ignore
>>>> +               }
>>>> +       }
>>>> +
>>>> +       private String readAndParseCommand() throws IOException {
>>>> +               String command = null;
>>>> +               boolean loop = true;
>>>> +               boolean first = true;
>>>> +               while (loop) {
>>>> +                   checkInterrupt();
>>>> +                   String line = reader.readLine(first ? getPrompt() :
>>>> ">    ");
>>>> +                   if (line == null)
>>>> +                   {
>>>> +                       break;
>>>> +                   }
>>>> +                   if (command == null) {
>>>> +                       command = line;
>>>> +                   } else {
>>>> +                       command += " " + line;
>>>> +                   }
>>>> +                   reader.getHistory().replace(command);
>>>> +                   try {
>>>> +                       new Parser(command).program();
>>>> +                       loop = false;
>>>> +                   } catch (Exception e) {
>>>> +                       loop = true;
>>>> +                       first = false;
>>>> +                   }
>>>> +               }
>>>> +               return command;
>>>> +       }
>>>> +
>>>> +       private void executeScript(String scriptFileName) {
>>>> +               if (scriptFileName != null) {
>>>> +            Reader r = null;
>>>> +            try {
>>>> +                File scriptFile = new File(scriptFileName);
>>>> +                r = new InputStreamReader(new
>>>> FileInputStream(scriptFile));
>>>> +                CharArrayWriter w = new CharArrayWriter();
>>>> +                int n;
>>>> +                char[] buf = new char[8192];
>>>> +                while ((n = r.read(buf))>    0) {
>>>> +                    w.write(buf, 0, n);
>>>> +                }
>>>> +                session.execute(new String(w.toCharArray()));
>>>> +            } catch (Exception e) {
>>>> +                LOGGER.debug("Error in initialization script", e);
>>>> +                System.err.println("Error in initialization script: " +
>>>> e.getMessage());
>>>> +            } finally {
>>>> +                if (r != null) {
>>>> +                    try {
>>>> +                        r.close();
>>>> +                    } catch (IOException e) {
>>>> +                        // Ignore
>>>> +                    }
>>>> +                }
>>>> +            }
>>>> +        }
>>>> +       }
>>>> +
>>>>      protected boolean getBoolean(String name) {
>>>>          Object s = session.get(name);
>>>>          if (s == null) {
>>>>
>>>>
>>>
>>
>> --
>>
>> Christian Schneider
>> http://www.liquid-reality.de
>>
>> Open Source Architect
>> Talend Application Integration Division http://www.talend.com
>>
>
>


-- 

Christian Schneider
http://www.liquid-reality.de

Open Source Architect
Talend Application Integration Division http://www.talend.com


Re: svn commit: r1132688 - in /karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console: completer/CommandsCompleter.java jline/Console.java

Posted by Guillaume Nodet <gn...@gmail.com>.
Not sure to understand what you mean. When you call getTarget, the
underlying code [1] calls context.getService
so you need to call ungetTarget to release this reference.

[1] https://github.com/apache/felix/blob/trunk/gogo/runtime/src/main/java/org/apache/felix/gogo/runtime/CommandProxy.java#L48

On Fri, Feb 17, 2012 at 12:36, Christian Schneider
<ch...@die-schneider.net> wrote:
> Hi Guillaume,
>
> I just checked the class you mentioned. It does not seem to contain
> context.getService anymore. So I think the code already looks good now.
>
> Christian
>
> Am 16.02.2012 10:53, schrieb Guillaume Nodet:
>>
>> Could you please try to fix the changes you've made ?   You need to
>> call unget in a finally block in order to release the service
>> reference.
>> In case you wondered why it was coded that way, the getTarget and
>> ungetTarget methods did not exists at that time ... so I had to
>> fallback to reflection.
>>
>> On Mon, Jun 6, 2011 at 18:05,<cs...@apache.org>  wrote:
>>>
>>> Author: cschneider
>>> Date: Mon Jun  6 16:05:00 2011
>>> New Revision: 1132688
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1132688&view=rev
>>> Log:
>>> Refactorings in CommandsCompleter and Console
>>>
>>> Modified:
>>>
>>>  karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>>
>>>  karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>>
>>> Modified:
>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>> URL:
>>> http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>> (original)
>>> +++
>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>> Mon Jun  6 16:05:00 2011
>>> @@ -18,7 +18,6 @@
>>>  */
>>>  package org.apache.karaf.shell.console.completer;
>>>
>>> -import java.lang.reflect.Field;
>>>  import java.util.ArrayList;
>>>  import java.util.Collections;
>>>  import java.util.HashSet;
>>> @@ -31,10 +30,7 @@ import org.apache.felix.gogo.runtime.Com
>>>  import org.apache.felix.gogo.runtime.CommandSessionImpl;
>>>  import org.apache.felix.service.command.CommandSession;
>>>  import org.apache.felix.service.command.Function;
>>> -import org.apache.karaf.shell.console.CompletableFunction;
>>>  import org.apache.karaf.shell.console.Completer;
>>> -import org.osgi.framework.BundleContext;
>>> -import org.osgi.framework.ServiceReference;
>>>
>>>  /**
>>>  * Like the {@link
>>> org.apache.karaf.shell.console.completer.CommandsCompleter} but does not use
>>> OSGi but is
>>> @@ -100,26 +96,17 @@ public class CommandsCompleter implement
>>>     }
>>>
>>>     protected Function unProxy(Function function) {
>>> -        try {
>>> -            if (function instanceof CommandProxy) {
>>> -                Field contextField =
>>> function.getClass().getDeclaredField("context");
>>> -                Field referenceField =
>>> function.getClass().getDeclaredField("reference");
>>> -                contextField.setAccessible(true);
>>> -                referenceField.setAccessible(true);
>>> -                BundleContext context = (BundleContext)
>>> contextField.get(function);
>>> -                ServiceReference reference = (ServiceReference)
>>> referenceField.get(function);
>>> -                Object target = context.getService(reference);
>>> -                try {
>>> -                    if (target instanceof Function) {
>>> -                        function = (Function) target;
>>> -                    }
>>> -                } finally {
>>> -                    context.ungetService(reference);
>>> -                }
>>> -            }
>>> -        } catch (Throwable t) {
>>> -        }
>>> -        return function;
>>> +       if (function instanceof CommandProxy) {
>>> +               CommandProxy proxy = (CommandProxy) function;
>>> +               Object target = proxy.getTarget();
>>> +                       if (target instanceof Function) {
>>> +                               return (Function) target;
>>> +                       } else {
>>> +                               return function;
>>> +                       }
>>> +       } else {
>>> +               return function;
>>> +       }
>>>     }
>>>
>>>  }
>>>
>>> Modified:
>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>> URL:
>>> http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>> (original)
>>> +++
>>> karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>> Mon Jun  6 16:05:00 2011
>>> @@ -182,57 +182,10 @@ public class Console implements Runnable
>>>         welcome();
>>>         setSessionProperties();
>>>         String scriptFileName = System.getProperty(SHELL_INIT_SCRIPT);
>>> -        if (scriptFileName != null) {
>>> -            Reader r = null;
>>> -            try {
>>> -                File scriptFile = new File(scriptFileName);
>>> -                r = new InputStreamReader(new
>>> FileInputStream(scriptFile));
>>> -                CharArrayWriter w = new CharArrayWriter();
>>> -                int n;
>>> -                char[] buf = new char[8192];
>>> -                while ((n = r.read(buf))>  0) {
>>> -                    w.write(buf, 0, n);
>>> -                }
>>> -                session.execute(new String(w.toCharArray()));
>>> -            } catch (Exception e) {
>>> -                LOGGER.debug("Error in initialization script", e);
>>> -                System.err.println("Error in initialization script: " +
>>> e.getMessage());
>>> -            } finally {
>>> -                if (r != null) {
>>> -                    try {
>>> -                        r.close();
>>> -                    } catch (IOException e) {
>>> -                        // Ignore
>>> -                    }
>>> -                }
>>> -            }
>>> -        }
>>> +        executeScript(scriptFileName);
>>>         while (running) {
>>>             try {
>>> -                String command = null;
>>> -                boolean loop = true;
>>> -                boolean first = true;
>>> -                while (loop) {
>>> -                    checkInterrupt();
>>> -                    String line = reader.readLine(first ? getPrompt() :
>>> ">  ");
>>> -                    if (line == null)
>>> -                    {
>>> -                        break;
>>> -                    }
>>> -                    if (command == null) {
>>> -                        command = line;
>>> -                    } else {
>>> -                        command += " " + line;
>>> -                    }
>>> -                    reader.getHistory().replace(command);
>>> -                    try {
>>> -                        new Parser(command).program();
>>> -                        loop = false;
>>> -                    } catch (Exception e) {
>>> -                        loop = true;
>>> -                        first = false;
>>> -                    }
>>> -                }
>>> +                String command = readAndParseCommand();
>>>                 if (command == null) {
>>>                     break;
>>>                 }
>>> @@ -254,35 +207,7 @@ public class Console implements Runnable
>>>             }
>>>             catch (Throwable t)
>>>             {
>>> -                try {
>>> -                    LOGGER.info("Exception caught while executing
>>> command", t);
>>> -                    session.put(LAST_EXCEPTION, t);
>>> -                    if (t instanceof CommandException) {
>>> -                        session.getConsole().println(((CommandException)
>>> t).getNiceHelp());
>>> -                    } else if (t instanceof CommandNotFoundException) {
>>> -                        String str = Ansi.ansi()
>>> -                            .fg(Ansi.Color.RED)
>>> -                            .a("Command not found: ")
>>> -                            .a(Ansi.Attribute.INTENSITY_BOLD)
>>> -                            .a(((CommandNotFoundException)
>>> t).getCommand())
>>> -                            .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>>> -                            .fg(Ansi.Color.DEFAULT).toString();
>>> -                        session.getConsole().println(str);
>>> -                    }
>>> -                    if ( getBoolean(PRINT_STACK_TRACES)) {
>>> -
>>>  session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>> -                        t.printStackTrace(session.getConsole());
>>> -
>>>  session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>> -                    }
>>> -                    else if (!(t instanceof CommandException)&&  !(t
>>> instanceof CommandNotFoundException)) {
>>>
>>> -
>>>  session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>> -                        session.getConsole().println("Error executing
>>> command: "
>>> -                                + (t.getMessage() != null ?
>>> t.getMessage() : t.getClass().getName()));
>>> -
>>>  session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>> -                    }
>>> -                } catch (Exception ignore) {
>>> -                        // ignore
>>> -                }
>>> +                logException(t);
>>>             }
>>>         }
>>>         close();
>>> @@ -293,6 +218,94 @@ public class Console implements Runnable
>>>         }
>>>     }
>>>
>>> +       private void logException(Throwable t) {
>>> +               try {
>>> +                   LOGGER.info("Exception caught while executing
>>> command", t);
>>> +                   session.put(LAST_EXCEPTION, t);
>>> +                   if (t instanceof CommandException) {
>>> +                       session.getConsole().println(((CommandException)
>>> t).getNiceHelp());
>>> +                   } else if (t instanceof CommandNotFoundException) {
>>> +                       String str = Ansi.ansi()
>>> +                           .fg(Ansi.Color.RED)
>>> +                           .a("Command not found: ")
>>> +                           .a(Ansi.Attribute.INTENSITY_BOLD)
>>> +                           .a(((CommandNotFoundException)
>>> t).getCommand())
>>> +                           .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>>> +                           .fg(Ansi.Color.DEFAULT).toString();
>>> +                       session.getConsole().println(str);
>>> +                   }
>>> +                   if ( getBoolean(PRINT_STACK_TRACES)) {
>>> +
>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>> +                       t.printStackTrace(session.getConsole());
>>> +
>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>> +                   }
>>> +                   else if (!(t instanceof CommandException)&&  !(t
>>> instanceof CommandNotFoundException)) {
>>>
>>> +
>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>>> +                       session.getConsole().println("Error executing
>>> command: "
>>> +                               + (t.getMessage() != null ?
>>> t.getMessage() : t.getClass().getName()));
>>> +
>>> session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>>> +                   }
>>> +               } catch (Exception ignore) {
>>> +                       // ignore
>>> +               }
>>> +       }
>>> +
>>> +       private String readAndParseCommand() throws IOException {
>>> +               String command = null;
>>> +               boolean loop = true;
>>> +               boolean first = true;
>>> +               while (loop) {
>>> +                   checkInterrupt();
>>> +                   String line = reader.readLine(first ? getPrompt() :
>>> ">  ");
>>> +                   if (line == null)
>>> +                   {
>>> +                       break;
>>> +                   }
>>> +                   if (command == null) {
>>> +                       command = line;
>>> +                   } else {
>>> +                       command += " " + line;
>>> +                   }
>>> +                   reader.getHistory().replace(command);
>>> +                   try {
>>> +                       new Parser(command).program();
>>> +                       loop = false;
>>> +                   } catch (Exception e) {
>>> +                       loop = true;
>>> +                       first = false;
>>> +                   }
>>> +               }
>>> +               return command;
>>> +       }
>>> +
>>> +       private void executeScript(String scriptFileName) {
>>> +               if (scriptFileName != null) {
>>> +            Reader r = null;
>>> +            try {
>>> +                File scriptFile = new File(scriptFileName);
>>> +                r = new InputStreamReader(new
>>> FileInputStream(scriptFile));
>>> +                CharArrayWriter w = new CharArrayWriter();
>>> +                int n;
>>> +                char[] buf = new char[8192];
>>> +                while ((n = r.read(buf))>  0) {
>>> +                    w.write(buf, 0, n);
>>> +                }
>>> +                session.execute(new String(w.toCharArray()));
>>> +            } catch (Exception e) {
>>> +                LOGGER.debug("Error in initialization script", e);
>>> +                System.err.println("Error in initialization script: " +
>>> e.getMessage());
>>> +            } finally {
>>> +                if (r != null) {
>>> +                    try {
>>> +                        r.close();
>>> +                    } catch (IOException e) {
>>> +                        // Ignore
>>> +                    }
>>> +                }
>>> +            }
>>> +        }
>>> +       }
>>> +
>>>     protected boolean getBoolean(String name) {
>>>         Object s = session.get(name);
>>>         if (s == null) {
>>>
>>>
>>
>>
>
>
> --
>
> Christian Schneider
> http://www.liquid-reality.de
>
> Open Source Architect
> Talend Application Integration Division http://www.talend.com
>



-- 
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com

Re: svn commit: r1132688 - in /karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console: completer/CommandsCompleter.java jline/Console.java

Posted by Christian Schneider <ch...@die-schneider.net>.
Hi Guillaume,

I just checked the class you mentioned. It does not seem to contain 
context.getService anymore. So I think the code already looks good now.

Christian

Am 16.02.2012 10:53, schrieb Guillaume Nodet:
> Could you please try to fix the changes you've made ?   You need to
> call unget in a finally block in order to release the service
> reference.
> In case you wondered why it was coded that way, the getTarget and
> ungetTarget methods did not exists at that time ... so I had to
> fallback to reflection.
>
> On Mon, Jun 6, 2011 at 18:05,<cs...@apache.org>  wrote:
>> Author: cschneider
>> Date: Mon Jun  6 16:05:00 2011
>> New Revision: 1132688
>>
>> URL: http://svn.apache.org/viewvc?rev=1132688&view=rev
>> Log:
>> Refactorings in CommandsCompleter and Console
>>
>> Modified:
>>     karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>>     karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>>
>> Modified: karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
>> URL: http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>> ==============================================================================
>> --- karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java (original)
>> +++ karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java Mon Jun  6 16:05:00 2011
>> @@ -18,7 +18,6 @@
>>   */
>>   package org.apache.karaf.shell.console.completer;
>>
>> -import java.lang.reflect.Field;
>>   import java.util.ArrayList;
>>   import java.util.Collections;
>>   import java.util.HashSet;
>> @@ -31,10 +30,7 @@ import org.apache.felix.gogo.runtime.Com
>>   import org.apache.felix.gogo.runtime.CommandSessionImpl;
>>   import org.apache.felix.service.command.CommandSession;
>>   import org.apache.felix.service.command.Function;
>> -import org.apache.karaf.shell.console.CompletableFunction;
>>   import org.apache.karaf.shell.console.Completer;
>> -import org.osgi.framework.BundleContext;
>> -import org.osgi.framework.ServiceReference;
>>
>>   /**
>>   * Like the {@link org.apache.karaf.shell.console.completer.CommandsCompleter} but does not use OSGi but is
>> @@ -100,26 +96,17 @@ public class CommandsCompleter implement
>>      }
>>
>>      protected Function unProxy(Function function) {
>> -        try {
>> -            if (function instanceof CommandProxy) {
>> -                Field contextField = function.getClass().getDeclaredField("context");
>> -                Field referenceField = function.getClass().getDeclaredField("reference");
>> -                contextField.setAccessible(true);
>> -                referenceField.setAccessible(true);
>> -                BundleContext context = (BundleContext) contextField.get(function);
>> -                ServiceReference reference = (ServiceReference) referenceField.get(function);
>> -                Object target = context.getService(reference);
>> -                try {
>> -                    if (target instanceof Function) {
>> -                        function = (Function) target;
>> -                    }
>> -                } finally {
>> -                    context.ungetService(reference);
>> -                }
>> -            }
>> -        } catch (Throwable t) {
>> -        }
>> -        return function;
>> +       if (function instanceof CommandProxy) {
>> +               CommandProxy proxy = (CommandProxy) function;
>> +               Object target = proxy.getTarget();
>> +                       if (target instanceof Function) {
>> +                               return (Function) target;
>> +                       } else {
>> +                               return function;
>> +                       }
>> +       } else {
>> +               return function;
>> +       }
>>      }
>>
>>   }
>>
>> Modified: karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java
>> URL: http://svn.apache.org/viewvc/karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java?rev=1132688&r1=1132687&r2=1132688&view=diff
>> ==============================================================================
>> --- karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java (original)
>> +++ karaf/trunk/shell/console/src/main/java/org/apache/karaf/shell/console/jline/Console.java Mon Jun  6 16:05:00 2011
>> @@ -182,57 +182,10 @@ public class Console implements Runnable
>>          welcome();
>>          setSessionProperties();
>>          String scriptFileName = System.getProperty(SHELL_INIT_SCRIPT);
>> -        if (scriptFileName != null) {
>> -            Reader r = null;
>> -            try {
>> -                File scriptFile = new File(scriptFileName);
>> -                r = new InputStreamReader(new FileInputStream(scriptFile));
>> -                CharArrayWriter w = new CharArrayWriter();
>> -                int n;
>> -                char[] buf = new char[8192];
>> -                while ((n = r.read(buf))>  0) {
>> -                    w.write(buf, 0, n);
>> -                }
>> -                session.execute(new String(w.toCharArray()));
>> -            } catch (Exception e) {
>> -                LOGGER.debug("Error in initialization script", e);
>> -                System.err.println("Error in initialization script: " + e.getMessage());
>> -            } finally {
>> -                if (r != null) {
>> -                    try {
>> -                        r.close();
>> -                    } catch (IOException e) {
>> -                        // Ignore
>> -                    }
>> -                }
>> -            }
>> -        }
>> +        executeScript(scriptFileName);
>>          while (running) {
>>              try {
>> -                String command = null;
>> -                boolean loop = true;
>> -                boolean first = true;
>> -                while (loop) {
>> -                    checkInterrupt();
>> -                    String line = reader.readLine(first ? getPrompt() : ">  ");
>> -                    if (line == null)
>> -                    {
>> -                        break;
>> -                    }
>> -                    if (command == null) {
>> -                        command = line;
>> -                    } else {
>> -                        command += " " + line;
>> -                    }
>> -                    reader.getHistory().replace(command);
>> -                    try {
>> -                        new Parser(command).program();
>> -                        loop = false;
>> -                    } catch (Exception e) {
>> -                        loop = true;
>> -                        first = false;
>> -                    }
>> -                }
>> +                String command = readAndParseCommand();
>>                  if (command == null) {
>>                      break;
>>                  }
>> @@ -254,35 +207,7 @@ public class Console implements Runnable
>>              }
>>              catch (Throwable t)
>>              {
>> -                try {
>> -                    LOGGER.info("Exception caught while executing command", t);
>> -                    session.put(LAST_EXCEPTION, t);
>> -                    if (t instanceof CommandException) {
>> -                        session.getConsole().println(((CommandException) t).getNiceHelp());
>> -                    } else if (t instanceof CommandNotFoundException) {
>> -                        String str = Ansi.ansi()
>> -                            .fg(Ansi.Color.RED)
>> -                            .a("Command not found: ")
>> -                            .a(Ansi.Attribute.INTENSITY_BOLD)
>> -                            .a(((CommandNotFoundException) t).getCommand())
>> -                            .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>> -                            .fg(Ansi.Color.DEFAULT).toString();
>> -                        session.getConsole().println(str);
>> -                    }
>> -                    if ( getBoolean(PRINT_STACK_TRACES)) {
>> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>> -                        t.printStackTrace(session.getConsole());
>> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>> -                    }
>> -                    else if (!(t instanceof CommandException)&&  !(t instanceof CommandNotFoundException)) {
>> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>> -                        session.getConsole().println("Error executing command: "
>> -                                + (t.getMessage() != null ? t.getMessage() : t.getClass().getName()));
>> -                        session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>> -                    }
>> -                } catch (Exception ignore) {
>> -                        // ignore
>> -                }
>> +                logException(t);
>>              }
>>          }
>>          close();
>> @@ -293,6 +218,94 @@ public class Console implements Runnable
>>          }
>>      }
>>
>> +       private void logException(Throwable t) {
>> +               try {
>> +                   LOGGER.info("Exception caught while executing command", t);
>> +                   session.put(LAST_EXCEPTION, t);
>> +                   if (t instanceof CommandException) {
>> +                       session.getConsole().println(((CommandException) t).getNiceHelp());
>> +                   } else if (t instanceof CommandNotFoundException) {
>> +                       String str = Ansi.ansi()
>> +                           .fg(Ansi.Color.RED)
>> +                           .a("Command not found: ")
>> +                           .a(Ansi.Attribute.INTENSITY_BOLD)
>> +                           .a(((CommandNotFoundException) t).getCommand())
>> +                           .a(Ansi.Attribute.INTENSITY_BOLD_OFF)
>> +                           .fg(Ansi.Color.DEFAULT).toString();
>> +                       session.getConsole().println(str);
>> +                   }
>> +                   if ( getBoolean(PRINT_STACK_TRACES)) {
>> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>> +                       t.printStackTrace(session.getConsole());
>> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>> +                   }
>> +                   else if (!(t instanceof CommandException)&&  !(t instanceof CommandNotFoundException)) {
>> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.RED).toString());
>> +                       session.getConsole().println("Error executing command: "
>> +                               + (t.getMessage() != null ? t.getMessage() : t.getClass().getName()));
>> +                       session.getConsole().print(Ansi.ansi().fg(Ansi.Color.DEFAULT).toString());
>> +                   }
>> +               } catch (Exception ignore) {
>> +                       // ignore
>> +               }
>> +       }
>> +
>> +       private String readAndParseCommand() throws IOException {
>> +               String command = null;
>> +               boolean loop = true;
>> +               boolean first = true;
>> +               while (loop) {
>> +                   checkInterrupt();
>> +                   String line = reader.readLine(first ? getPrompt() : ">  ");
>> +                   if (line == null)
>> +                   {
>> +                       break;
>> +                   }
>> +                   if (command == null) {
>> +                       command = line;
>> +                   } else {
>> +                       command += " " + line;
>> +                   }
>> +                   reader.getHistory().replace(command);
>> +                   try {
>> +                       new Parser(command).program();
>> +                       loop = false;
>> +                   } catch (Exception e) {
>> +                       loop = true;
>> +                       first = false;
>> +                   }
>> +               }
>> +               return command;
>> +       }
>> +
>> +       private void executeScript(String scriptFileName) {
>> +               if (scriptFileName != null) {
>> +            Reader r = null;
>> +            try {
>> +                File scriptFile = new File(scriptFileName);
>> +                r = new InputStreamReader(new FileInputStream(scriptFile));
>> +                CharArrayWriter w = new CharArrayWriter();
>> +                int n;
>> +                char[] buf = new char[8192];
>> +                while ((n = r.read(buf))>  0) {
>> +                    w.write(buf, 0, n);
>> +                }
>> +                session.execute(new String(w.toCharArray()));
>> +            } catch (Exception e) {
>> +                LOGGER.debug("Error in initialization script", e);
>> +                System.err.println("Error in initialization script: " + e.getMessage());
>> +            } finally {
>> +                if (r != null) {
>> +                    try {
>> +                        r.close();
>> +                    } catch (IOException e) {
>> +                        // Ignore
>> +                    }
>> +                }
>> +            }
>> +        }
>> +       }
>> +
>>      protected boolean getBoolean(String name) {
>>          Object s = session.get(name);
>>          if (s == null) {
>>
>>
>
>


-- 

Christian Schneider
http://www.liquid-reality.de

Open Source Architect
Talend Application Integration Division http://www.talend.com