[RFC][icedtea-web] fixes to reproducer engine
Jiri Vanek
jvanek at redhat.com
Thu Mar 22 12:23:08 PDT 2012
On 03/22/2012 03:04 PM, Deepak Bhole wrote:
> * 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.:
Yap, I hope most of them (omg so much :( ) catch.
>
> + 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();
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixesToJnlpReproducersEngine2.diff
Type: text/x-patch
Size: 15416 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120322/92105fe9/fixesToJnlpReproducersEngine2.diff
More information about the distro-pkg-dev
mailing list