[RFC][icedtea-web] fixes to reproducer engine
Deepak Bhole
dbhole at redhat.com
Thu Mar 22 07:04:38 PDT 2012
* Jiri Vanek <jvanek at redhat.com> [2012-03-21 10:22]:
> Hi!
>
> As I wrote yesterday, I have found some bugs inside jnlp main
> engine. This patch is fixing them, and I would like to push it to
> head and 1.2 (with minor changes).
>
> Any comments?
>
Hi Jiri,
Can you please repost with indentations fixed? e.g.:
+ while (!t.isDestoyed()){
+ Thread.sleep(100);
+ }
There is no space before the { when there should be. See guidelines here:
http://icedtea.classpath.org/wiki/IcedTea-Web#Code_style
Also, changelog has spacing issues -- there should be no spaces before any of
the words on a new line, only a single tab e.g
Fixing issue when process was not launched at all and when was killed but
left behind living/hanging, fixing mime-types
should be:
Fixing issue when process was not launched at all and when was killed but
left behind living/hanging, fixing mime-types
i.e. only a tab before "left", not a tab and space.
Cheers,
Deepak
> Thanx!
> J.
> diff -r 89609d0a4e1c ChangeLog
> --- a/ChangeLog Mon Mar 19 14:37:03 2012 -0400
> +++ b/ChangeLog Wed Mar 21 15:15:48 2012 +0100
> @@ -1,3 +1,20 @@
> +2012-03-21 Jiri Vanek <jvanek at redhat.com>
> +
> + Fixing issue when process was not launched at all and when was killed but
> + left behind living/hanging, fixing mime-types
> + * tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java:
> + (getContentOfStream) this method overloaded with possibility to specify
> + encoding (I needed to set it to ASCII in one test)
> + (deadlyException) field introduced in ThreadedProcess to record exception
> + caused by impassibility of launching the process. And so process have
> + been null without any sign why.
> + (TinyHttpdImpl) now correctly returns known mime types
> + (ProcessAssasin) can now skip or smoothly (and finally correctly)
> + destroy its process, and all his logging messages were done null-proof
> + (as deadlyException now allows)
> + Asynchronous (ContentReader) have been silenced when complaining about
> + closed streams by Assassin.
> +
> 2012-03-19 Danesh Dadachanji <ddadacha at redhat.com>
>
> Fix failing unit test missing title/vendor tags in the JNLP stream.
> diff -r 89609d0a4e1c tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java
> --- a/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java Mon Mar 19 14:37:03 2012 -0400
> +++ b/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java Wed Mar 21 15:15:48 2012 +0100
> @@ -254,7 +254,7 @@
>
> @Test
> public void testsProcessResultFiltering() throws Exception {
> - ProcessResult pn = new ProcessResult(null, null, null, true, 0);
> + ProcessResult pn = new ProcessResult(null, null, null, true, 0,null);
> Assert.assertNull(pn.notFilteredStdout);
> Assert.assertNull(pn.stdout);
> Assert.assertNull(pn.stderr);
> @@ -273,12 +273,12 @@
> "test stage 1\n"
> + "test stage 2\n"
> + "test stage 3\n";
> - ProcessResult p2 = new ProcessResult(fakeOut2, fakeOut2, null, true, 0);
> + ProcessResult p2 = new ProcessResult(fakeOut2, fakeOut2, null, true, 0,null);
> Assert.assertEquals(p2.notFilteredStdout, fakeOut2);
> Assert.assertEquals(p2.stdout, filteredOut2);
> Assert.assertEquals(p2.stderr, fakeOut2);
> fakeOut2+="\n";
> - p2 = new ProcessResult(fakeOut2, fakeOut2, null, true, 0);
> + p2 = new ProcessResult(fakeOut2, fakeOut2, null, true, 0,null);
> Assert.assertEquals(p2.notFilteredStdout, fakeOut2);
> Assert.assertEquals(p2.stdout, filteredOut2);
> Assert.assertEquals(p2.stderr, fakeOut2);
> @@ -300,13 +300,13 @@
> + "test stage 2\n"
> + "test stage 3\n"
> + "test ends";
> - ProcessResult p = new ProcessResult(fakeOut, fakeOut, null, true, 0);
> + ProcessResult p = new ProcessResult(fakeOut, fakeOut, null, true, 0,null);
> Assert.assertEquals(p.notFilteredStdout, fakeOut);
> Assert.assertEquals(p.stdout, filteredOut);
> Assert.assertEquals(p.stderr, fakeOut);
> fakeOut+="\n";
> filteredOut+="\n";
> - p = new ProcessResult(fakeOut, fakeOut, null, true, 0);
> + p = new ProcessResult(fakeOut, fakeOut, null, true, 0,null);
> Assert.assertEquals(p.notFilteredStdout, fakeOut);
> Assert.assertEquals(p.stdout, filteredOut);
> Assert.assertEquals(p.stderr, fakeOut);
> @@ -473,9 +473,9 @@
> * @return stream as string
> * @throws IOException if connection cant be established or resource do not exists
> */
> - public static String getContentOfStream(InputStream is) throws IOException {
> + public static String getContentOfStream(InputStream is,String encoding) throws IOException {
> try {
> - BufferedReader br = new BufferedReader(new InputStreamReader(is, "UTF-8"));
> + BufferedReader br = new BufferedReader(new InputStreamReader(is, encoding));
> StringBuilder sb = new StringBuilder();
> while (true) {
> String s = br.readLine();
> @@ -492,6 +492,18 @@
>
> }
>
> + /**
> + * utility method which can read from any stream as one long String
> + *
> + * @param input stream
> + * @return stream as string
> + * @throws IOException if connection cant be established or resource do not exists
> + */
> + public static String getContentOfStream(InputStream is) throws IOException {
> + return getContentOfStream(is,"UTF-8");
> +
> + }
> +
> /**
> * utility method which can read bytes of resource from any url
> *
> @@ -703,9 +715,13 @@
> ProcessAssasin pa = new ProcessAssasin(t, PROCESS_TIMEOUT);
> pa.start();
> t.start();
> - while (t.getP() == null) {
> + while (t.getP() == null && t.deadlyException==null) {
> Thread.sleep(100);
> }
> + if (t.deadlyException!=null){
> + pa.setCanRun(false);
> + return new ProcessResult("", "", null, true, Integer.MIN_VALUE,t.deadlyException);
> + }
> ContentReader crs = new ContentReader(t.getP().getInputStream(),stdoutl);
> ContentReader cre = new ContentReader(t.getP().getErrorStream(),stderrl);
>
> @@ -719,10 +735,14 @@
> while (t.isRunning()) {
> Thread.sleep(100);
> }
> +
> + while (!t.isDestoyed()){
> + Thread.sleep(100);
> + }
> pa.setCanRun(false);
> // System.out.println(t.getP().exitValue()); when process is killed, this throws exception
>
> - return new ProcessResult(crs.getContent(), cre.getContent(), t.getP(), pa.wasTerminated(), t.getExitCode());
> + return new ProcessResult(crs.getContent(), cre.getContent(), t.getP(), pa.wasTerminated(), t.getExitCode(),null);
> }
>
> /**
> @@ -737,6 +757,22 @@
> Integer exitCode;
> Boolean running;
> File dir;
> + Throwable deadlyException=null;
> + /*
> + * before removing this "useless" variable
> + * check DeadLockTestTest.testDeadLockTestTerminated2
> + */
> + private boolean destoyed=false;
> +
> + public boolean isDestoyed() {
> + return destoyed;
> + }
> +
> + public void setDestoyed(boolean destoyed) {
> + this.destoyed = destoyed;
> + }
> +
> +
>
> public Boolean isRunning() {
> return running;
> @@ -785,13 +821,21 @@
> }else{
> p = r.exec(args.toArray(new String[0]),new String[0], dir);
> }
> + try{
> exitCode = p.waitFor();
> Thread.sleep(500);//this is giving to fastly done proecesses's e/o readers time to read all. I would like to know better solution :-/
> - } catch (Exception ex) {
> + }finally{
> + destoyed=true;
> + }
> + } catch (Exception ex) {
> if (ex instanceof InterruptedException) {
> //add to the set of terminated threadedproceses
> + deadlyException=ex;
> terminated.add(this);
> } else {
> + //happens when nonexisting process is launched, is causing p null!
> + terminated.add(this);
> + deadlyException=ex;
> throw new RuntimeException(ex);
> }
> } finally {
> @@ -937,8 +981,16 @@
> byte[] b = new byte[l];
> FileInputStream f = new FileInputStream(pp);
> f.read(b);
> + String content="";
> + if (p.toLowerCase().endsWith(".jnlp")){
> + content="Content-Type: application/x-java-jnlp-file\n";
> + }else if (p.toLowerCase().endsWith(".html")){
> + content="Content-Type: text/html\n";
> + }else if (p.toLowerCase().endsWith(".jar")){
> + content="Content-Type: application/x-jar\n";
> + }
> o.writeBytes("HTTP/1.0 200 OK\nConten"
> - + "t-Length:" + l + "\n\n");
> + + "t-Length:" + l +"\n"+content+"\n");
> if (op.startsWith(XSX)){
> byte[][] bb=ServerAccess.splitArray(b,10);
> for (int j = 0; j < bb.length; j++) {
> @@ -1061,6 +1113,13 @@
> //false == is disabled:(
> private boolean canRun = true;
> private boolean wasTerminated = false;
> + /**
> + * if this is true, then process is not destroyed after is timeouted run, but just left to its own destiny.
> + * Its stdout/err is no longer recorded, and it is leaking system resources until it dies by itslef
> + * The contorl is returned to main thread with all informations recorded untill now.
> + * You will be able to listen to std out from listeners still
> + */
> + private boolean skipInstedOfDesroy=false;
>
> /**
> *
> @@ -1074,9 +1133,25 @@
>
> }
>
> + public ProcessAssasin(ThreadedProcess p, long timeout,boolean skipInstedOfDesroy) {
> + this.p = (p);
> + this.timeout = timeout;
> + this.skipInstedOfDesroy=skipInstedOfDesroy;
> +
> +
> + }
> +
> public void setCanRun(boolean canRun) {
> this.canRun = canRun;
> - System.err.println("Stopping assasin for" + p.toString() + " " + p.getP().toString() + " " + p.getCommandLine() + ": ");
> + if (p != null) {
> + if (p.getP() != null) {
> + System.err.println("Stopping assasin for" + p.toString() + " " + p.getP().toString() + " " + p.getCommandLine() + ": ");
> + } else {
> + System.err.println("Stopping assasin for" + p.toString() + " " + p.getCommandLine() + ": ");
> + }
> + }else{
> + System.err.println("Stopping assasin for null job: ");
> + }
> System.err.flush();
> }
>
> @@ -1088,6 +1163,16 @@
> return wasTerminated;
> }
>
> + public void setSkipInstedOfDesroy(boolean skipInstedOfDesroy) {
> + this.skipInstedOfDesroy = skipInstedOfDesroy;
> + }
> +
> + public boolean isSkipInstedOfDesroy() {
> + return skipInstedOfDesroy;
> + }
> +
> +
> +
> @Override
> public void run() {
>
> @@ -1099,17 +1184,45 @@
> //System.out.println(time - startTime);
> //System.out.println((time - startTime) > timeout);
> if ((time - startTime) > timeout) {
> - System.err.println("Timeouted " + p.toString() + " " + p.getP().toString() + " .. killing " + p.getCommandLine() + ": ");
> + try{
> + if (p!=null){
> + if (p.getP()!=null){System.err.println("Timeouted " +
> + p.toString() + " " +
> + p.getP().toString() +
> + " .. killing " +
> + p.getCommandLine() + ": ");
> + }else{System.err.println("Timeouted " +
> + p.toString() + " " +
> + "null .. killing " +
> + p.getCommandLine() + ": ");
> + }
> System.err.flush();
> wasTerminated = true;
> p.interrupt();
> while (!terminated.contains(p)) {
> Thread.sleep(100);
> }
> - //p.p.destroy()??
> - System.err.println("Timeouted " + p.toString() + " " + p.getP().toString() + " .. killed " + p.getCommandLine());
> + if (p.getP()!=null){
> + try{
> + if (!skipInstedOfDesroy) p.getP().destroy();
> + }catch(Throwable ex){
> + if (p.deadlyException==null) p.deadlyException=ex;
> + ex.printStackTrace();
> + }
> + }
> + if (p.getP()!=null){
> + System.err.println("Timeouted " + p.toString() + " " + p.getP().toString() + " .. killed " + p.getCommandLine());
> + }else{
> + System.err.println("Timeouted " + p.toString() + " null .. killed " + p.getCommandLine());
> + }
> System.err.flush();
> + }else{
> + System.err.println("Timeouted null job");
> + }
> break;
> + }finally{
> + p.setDestoyed(true);
> + }
>
>
> }
> @@ -1118,7 +1231,15 @@
> ex.printStackTrace();
> }
> }
> - System.err.println("assasin for" + p.toString() + " " + p.getP().toString() + " .. done " + p.getCommandLine() + " termination " + wasTerminated);
> + if (p!=null){
> + if (p.getP()!=null){
> + System.err.println("assasin for" + p.toString() + " " + p.getP().toString() + " .. done " + p.getCommandLine() + " termination " + wasTerminated);
> + }else{
> + System.err.println("assasin for" + p.toString() + " null .. done " + p.getCommandLine() + " termination " + wasTerminated);
> + }
> + }else{
> + System.err.println("assasin for non exisitng job termination " + wasTerminated);
> + }
> System.err.flush();
> }
> }
> @@ -1134,8 +1255,12 @@
> public final Process process;
> public final Integer returnValue;
> public final boolean wasTerminated;
> + /*
> + * possible exception which caused Process not to be launched
> + */
> + public final Throwable deadlyException;
>
> - public ProcessResult(String stdout, String stderr, Process process, boolean wasTerminated, Integer r) {
> + public ProcessResult(String stdout, String stderr, Process process, boolean wasTerminated, Integer r,Throwable deadlyException) {
> this.notFilteredStdout = stdout;
> if (stdout == null) {
> this.stdout = null;
> @@ -1146,6 +1271,7 @@
> this.process = process;
> this.wasTerminated = wasTerminated;
> this.returnValue = r;
> + this.deadlyException=deadlyException;
> }
> }
>
> @@ -1219,8 +1345,10 @@
>
> }
> //do not want to bother output with terminations
> + //mostly compaling when assasin kill the process about StreamClosed
> } catch (Exception ex) {
> - ex.printStackTrace();
> + // ex.printStackTrace();
> + // System.err.flush();
> } finally {
> try {
> is.close();
More information about the distro-pkg-dev
mailing list