[rfc][icedtea-web] make console aware of plugin messages
Pavel Tisnovsky
ptisnovs at redhat.com
Thu Dec 12 09:15:29 PST 2013
Hi Jiri,
> And none of above issue is fixed when used together with rewritten console.
> I swear They were not presented when I was splitting the patch, so I'm going to rerun them again
> tomorow, and fix them as another chnageset if needed.
ok then, please don't forget and ok to push these changes.
Thank you very much,
Pavel
----- Jiri Vanek <jvanek at redhat.com> wrote:
> On 12/11/2013 04:57 PM, Pavel Tisnovsky wrote:
> > Hi Jiri,
> >
> > this patch looks good in overall (mainly the synchronization during pushing/popping strings to a pipe!).
>
> Thanx!
>
> > I have just minor objections:
> >
> > netx/net/sourceforge/jnlp/util/logging/JavaConsole.java:
> >
> > the code:
> > while (true) {
> > try{
> > String s = br.readLine();
> > if (s == null) {
> > break;
> > }
> > processPluginMessage(s);
> >
> > might be rewritten as:
> > while ((String s = br.readLine()) != null) {
> > processPluginMessage(s);
> > }
>
> Oh no, I hate this style :( //not fixed
> >
> > OutputController.getLogger().log("Ended processing of plugin-debug-to-conosle " + file.getAbsolutePath());
> > ^^^^^^
> > OutputController.getLogger().log("Started processing of plugin-debug-to-conosle " + file.getAbsolutePath());\
>
> sure, thanx! Also I made Thread named.
> > ^^^^^^
> > netx/net/sourceforge/jnlp/util/logging/LogConfig.java Fri Nov 29 17:30:50 2013 +0100
> > + * logging stuff fmay be interested in used config
> > ^^^
>
> fixed
> >
> > That code smells a bit :-) but now I know why U did it - to use the same method in a console with
> > selectable output format:
> > + public String toString() {
> > + return toString(true, true, true, true, true, true, true);
> > + }
> >
> >
> yah :)
>
> > plugin/icedteanp/IcedTeaNPPlugin.cc:
> >
> > IMHO it's better to explicitly initialize those variables to NULL, even if it's not scrictly necessary:
> > +// Applet viewer debug pipe name.
> > +gchar* debug_pipe_name;
> > +
>
> done
> >
> > I'm not sure if it's better to flush the pipe before reconecting the C and Java part:
> > + if (plugin_debug_to_console){
> > + //jvm_up is now false
> > + if (g_io_channel_shutdown (debug_to_appletviewer,
> >
>
> nope - in this time console is dead anyway. So I would rather not risk flush from different thread.
> >
> > plugin/icedteanp/IcedTeaNPPlugin.h
> > + nanosleep(&ts ,0); it might be time consuming, do you have any profiler
> > output showing if there is/is not a perf. problem here?
>
> Welll, I dont know. But it is worthy to work on as another changeset.
> >
> >
> > + snprintf(ldebug_message, MESSAGE_SIZE, "%s%s", ldebug_header, ldebug_body); \
> > + char ldebug_channel_message[MESSAGE_SIZE+50]; \
> > + struct timeval current_time; \
> > + gettimeofday (¤t_time, NULL);\
> > + if (jvm_up) { \
> > + sprintf(ldebug_channel_message,"%s %ld %s", "plugindebug", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message); \
> > ^^ should be snprintf(ldebug_message, MESSAGE_SIZE+50 or you migth create new constant (which is better)
> > + push_pre_init_messages(ldebug_channel_message); \
> > + } else { \
> > + sprintf(ldebug_channel_message,"%s %ld %s", "preinit_plugindebug", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message); \
> > ^^ should be snprintf(ldebug_message, MESSAGE_SIZE+50 or you migth create new constant (which is better)
>
> all four fixed, and constan introduced.
>
> > + push_pre_init_messages(ldebug_channel_message); \
> >
> > + struct timeval current_time; \
> > + gettimeofday (¤t_time, NULL);\
> > + if (jvm_up) { \
> > + sprintf(ldebug_channel_message,"%s %ld %s", "pluginerror", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message); \
> > + push_pre_init_messages(ldebug_channel_message); \
> > + } else { \
> > + sprintf(ldebug_channel_message,"%s %ld %s", "preinit_pluginerror", current_time.tv_sec*1000000L+current_time.tv_usec, ldebug_message); \
> > + push_pre_init_messages(ldebug_channel_message); \
> > dtto, a good candidate for refactoring :)
>
> true. Voulenteer O:) ?
> >
> > + OutputController.getLogger().log(OutputController.Level.ERROR_ALL, "Warning, althoug console is on, plugin debug conenctin do not exists. No plugn information will be dysplayed in console (only java ones).");
> > althoug <- typo
> > dysplayed <- typo
>
> fixed
> >
> >
>
> However, bunch of tests started to fail with one those two exceptions:
>
> eg java.lang.NullPointerException
> at LessVerboseTextListener.getAnnotation(LessVerboseTextListener.java:126)
> at LessVerboseTextListener.getRemote(LessVerboseTextListener.java:147)
> at LessVerboseTextListener.printRemote(LessVerboseTextListener.java:153)
> at LessVerboseTextListener.testFailure(LessVerboseTextListener.java:52)
> at org.junit.runner.notification.RunNotifier$4.notifyListener(RunNotifier.java:139)
> at org.junit.runner.notification.RunNotifier$SafeNotifier.run(RunNotifier.java:61)
> at org.junit.runner.notification.RunNotifier.fireTestFailures(RunNotifier.java:134)
> at org.junit.runner.notification.RunNotifier.fireTestFailure(RunNotifier.java:128)
> at org.junit.internal.runners.model.EachTestNotifier.addFailure(EachTestNotifier.java:23)
> at
> org.junit.internal.runners.model.EachTestNotifier.addMultipleFailureException(EachTestNotifier.java:30)
> at org.junit.internal.runners.model.EachTestNotifier.addFailure(EachTestNotifier.java:21)
> at org.junit.runners.ParentRunner.run(ParentRunner.java:315)
> at org.junit.runners.Suite.runChild(Suite.java:127)
> at org.junit.runners.Suite.runChild(Suite.java:26)
> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
> at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
> at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
> at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
> at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
> at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
> at org.junit.runner.JUnitCore.run(JUnitCore.java:160)
> at org.junit.runner.JUnitCore.run(JUnitCore.java:138)
> at org.junit.runner.JUnitCore.run(JUnitCore.java:117)
> at CommandLine.runMain(CommandLine.java:48)
> at CommandLine.runMainAndExit(CommandLine.java:28)
> at CommandLine.main(CommandLine.java:24)
>
>
> and
>
> Exception in thread "Thread-112" java.lang.NoClassDefFoundError: Could not initialize class
> sun.awt.X11.XToolkit
> at java.lang.Class.forName0(Native Method)
> at java.lang.Class.forName(Class.java:190)
> at java.awt.Toolkit$2.run(Toolkit.java:868)
> at java.security.AccessController.doPrivileged(Native Method)
> at java.awt.Toolkit.getDefaultToolkit(Toolkit.java:860)
> at javax.swing.UIManager.getSystemLookAndFeelClassName(UIManager.java:608)
> at net.sourceforge.jnlp.util.logging.JavaConsole.initialize(JavaConsole.java:164)
> at net.sourceforge.jnlp.util.logging.JavaConsole.<init>(JavaConsole.java:126)
> at net.sourceforge.jnlp.util.logging.JavaConsole.getConsole(JavaConsole.java:91)
> at net.sourceforge.jnlp.util.logging.OutputController.consume(OutputController.java:157)
> at net.sourceforge.jnlp.util.logging.OutputController.access$200(OutputController.java:46)
> at net.sourceforge.jnlp.util.logging.OutputController$1.run(OutputController.java:229)
> at java.lang.Thread.run(Thread.java:744)
>
>
> and one memory leek was detected
>
> ...
> Passed: get_scriptable_object
> NOTE: Next error expected
> Unknown plugin value requested.
> *** WARNING: NP_GetValue has a memory leak! 1 more operator 'new' allocations than 'delete's!
> Passed: NP_GetValue
> ...
>
>
> And none of above issue is fixed when used together with rewritten console.
> I swear They were not presented when I was splitting the patch, so I'm going to rerun them again
> tomorow, and fix them as another chnageset if needed.
More information about the distro-pkg-dev
mailing list