DaCapo benchmark triggers proclet related stack overflow

Project:JNode Shell
Component:Code
Category:bug report
Priority:critical
Assigned:Stephen Crawley
Status:closed
Description

Peter reported that he gets stack overflow errors when he tries to run the DaCapo benchmark from the JNode console. This is apparently a long-standing bug and is triggered when the benchmark is run without arguments ("Harness") or with arguments ("Harness -s small -n fop").

Peter was using the latest binary distro downloaded from http://dacapobench.org/

Here's a typical stack overflow. It suggests that DaCapo is doing something that is creating a loop in the proxy streams.

 Debug stacktrace: org.jnode.vm.VmStackReader::debugStackTrace
 , org.jnode.vm.Unsafe::debugStackTrace
 , org.jnode.vm.scheduler.VmThread::systemException
 , java.lang.Thread::currentThread
 , org.jnode.shell.proclet.ProcletProxyPrintStream::proxiedPrintStream
 , org.jnode.shell.proclet.ProcletProxyPrintStream::getRealStream
 , org.jnode.shell.proclet.ProcletProxyPrintStream::effectiveOutput
 , org.jnode.shell.proclet.AbstractProxyPrintStream::write
 , java.io.FilterOutputStream::write
 , dacapo.TeeOutputStream::write
 , java.io.PrintStream::write
 , org.jnode.shell.proclet.AbstractProxyPrintStream::write
 , java.io.FilterOutputStream::write
 , dacapo.TeeOutputStream::write
 , java.io.PrintStream::write
 , org.jnode.shell.proclet.AbstractProxyPrintStream::write
 , java.io.FilterOutputStream::write
 , dacapo.TeeOutputStream::write
 , java.io.PrintStream::write
 , org.jnode.shell.proclet.AbstractProxyPrintStream::write
 , java.io.FilterOutputStream::write
 , dacapo.TeeOutputStream::write
 , java.io.PrintStream::write
 ..........

#1

Some small updates on that issue:
I checked out the svn trunk version of DaCapo and modified class TeeOutputStream (online version). I simply removed all super.xxx() calls that were producing stack overflow exceptions.
Despite of the Stackoverflow I had some troubles compiling the sources. I simply removed all non-compiling benchmarks from the ant file. I did two different tests: 1.) use the resulting jar with Fabien's packager and 2.) made my own plugin out of the jar file.
Both had the same problem: If a benchmark is executed for the first time, it starts executing but never finishes and silently returns to the command line. If the same benchmark is executed a second time it throws an Exception.
I messed my build environment so atm I can't provide a proper stacktrace but afair it was related to some filesystem issues. If I removed all temp files before executing a benchmark a second time, the second run "works" just as the first one did.

At the moment I have no idea what the problem could be as I do not get any usefull output. Any inputs welcome and once my build env works again I'll supply some more infos.

#2

I've figured out what is going on, and I'm afraid there is no fix that I can think of.

Here's what the DaCapo is doing:

  1. It gets the value of System.out.
  2. It creates a new instance of 'TeePrintStream', passing System.out as a parameter to the constructor.
  3. It then calls System.setOut(PrintStream), to try to set System.out to the new Tee stream.
  4. Finally it tries to write something to System.out.

The third step is where things go wrong. The problem is that the 'proclet' mechanism works by replacing System.out with a proxy stream object that causes output to System.out to go to different places depending on the proclet context. In addition, it changes the behaviour of System.setOut(...) so that it doesn't change System.out but it changes where the proxy sends output to. Unfortunately, in this particular use-case, the end result is that the proxy writes to the Tee stream which writes to the old System.out which is ... the proxy. And so on until we fill the stack.

I can think of work-arounds, but unfortunately I cannot think of a definitive fix that will would without making a change to the DaCapo codebase. Here are some workarounds.

  1. Change DaCapo to not use the Tee... streams.
  2. Change JNode to not use the ProclectInvoker as the initial shell invoker. (The problem is that once JNode has activated the proclet mechanism within an Isolate, there is no way to deactivate it. You might be able to set the invoker to "thread" and launch a new console with "-i" to get a new Isolate, but I haven't tried this.)
  3. In the code that sets up the Tee stream do something like this:
         ProxyStream<?> proxy = (ProxyStream<?>) System.out;
         PrintStream out = (PrintStream) proxy.getRealStream();
         // 'out' above might need to be an OutputStream ...
         TeePrintStream tee = new TeePrintStream(out, ...);
         System.setOut(tee);
    

In the future, I hope to implement an IsolateInvoker that will allow people to launch applications like DaCapo in their own isolates. In the mean time, this kind of use-case is problematic.

#3

What dacapo is doing is perfectly legal Java code and I think it might be done by a lot of other nontrivial software. So the only acceptable solution to this problem is to support that usecase in JNode by default without any extra steps like using a different invoker than the default. Great attention must be paid to preserving the semantics of the standard APIs while we are doing various tricks behind the scenes.

#4

Agreed.

Are you still working on Isolate infrastructure? Do you have an ETA for completion? IMO, that is the only way to deal with this kind of use-case.

If you recall, I proposed and implemented proclets (in part) as a way to make progress in the shell area while waiting for the Isolate implementation to be completed.

#5

I would be fine with 1. and 3. Smiling
Regarding your debugging, did you use trunk or the latest stable release (Why I'm asking is because I wasn't able to compile trunk as noted above)?
Did you experience any of the other bugs/problems like me or did you "only" try to fix the overflow?

#6

I cannot say a speciffic date for this. In some situations which include most of the non graphical programs isolates might be already usable. Probably we could start using them more widely and gain more experience with them. Before an isolate based invoker I'm thinking about a speciffic command for isolates and probably in perespective the current java command could be used for that so that it would start the specified class in a new isolate. This would be in corcondace with the java command on other operating systems where it always starts up a separate JVM hence achieving isolation.

#7

In reply to comment #5: I diagnosed the problem by reading the DaCapo code and figuring out (in my head) what the proclet mechanism would do. I didn't build or run DaCapo in the end, so I didn't encounter the problems you have been having with getting it to compile, etc.

As a suggestion, were you able to compile and run DaCapo using just a standard JDK? If you were not able to do this, the problem is clearly on the DaCapo side, and (IMO) it would be worth trying to contact the DaCapo authors.

#8

Status:active» won't fix

I don't think there is anything more to say on this issue. The problem is really an inherent limitation of the proclet approach. Marking issue as 'won't fix'.

#10

Priority:normal» critical
Status:won't fix» active

I have to reopen this issue for further investigation because it creates major problems.
At the current state jetty is rendered nearly unusable bcause if I start up jetty with the default invoker it produces stack overflow and if I start up jetty with the isolate invoker I cannot see the error stream. However in an isolated jetty some problems are produced and I cannot diagnose them and generally isolated execution is not stable enough yet for relying on it exclusively.

I think the bug must had been introduced in a not so old commit because jetty worked fine when I added it to the distribution one or one and a half year ago and it also worked for quite a while afterwards.

A possible solution could reinvestigate the commit which introduced the bug and find a better solution for the problem that the commit was trying to fix.

Heading towards the release I think this is a critical bug.

#11

The only "fix" I can think of is to go back to using the ThreadInvoker as the default invoker. Of course that will resurface the anomalies which caused us to make the ProcletInvoker the default in the first place.

It's your call Levente.

#12

Isn't it possible to set up the steams so to avoid such loops?

A key requirement of the API is that what you set System.setOut() the excatly same object should be available in System.out. I don't think you should try to capture the data of a custom system output stream by any means. In the JDK also it's possible to redicrect the output to palces completely different from the original target. In that case the application takes care of it and the JVM can do nothing about it.
The case that we should take care of is exactly this which produces the stack overflow. The application wraps the original System.out into something for more convenient use and then installs the wrapper stream as the global System.out of the application. In this case the goal is to capture everything that is sent to System.out but also keep the original destination of the original output stream.
I think the solution should handle this case first of all and don't care about the case when the application simply installs a stream to System.out which points to a file or buffer or any other location except the original stream. With such move the application can irecoverably lose the original output stream and that is ok. The outside world will just get nothing on that stream from the application because nothing is written to it.

So for the important case there could be a redirecting stream which follows the needs of the current proclet and it will redirect to the console or to a pipe depending on the calling conditions of the proclet. The point is that if the application sets a custom stream then that custom stream should be visible in System.out, and if the application wraps the original stream then the data will go to the right place bacause the redirecting stream is wrapped.

What do you think?

#13

A key requirement of the API is that what you set System.setOut() the exectaly same object should be available in System.out.

Proclets violate this requirement, and this is unavoidable. I remember pointing this out when I designed them.

IIRC, I looked at changing the System.setOut() code to avoid setting up proxy loops. The System.setOut code copes with this:

   System.setOut(System.out);

by noticing that 'System.out' is a proxy stream and resolving.
The problem is with Stream wrappers; i.e. where you do something like this:

    System.setOut(new PrintStream(System.out));

It is theoretically possible to detect that a proxy cycle is about to be created in a subset of cases; e.g. by reflection and "knowledge" of the internals of standard / JNode-specific Stream classes. However, I cannot see how to make setOut() do what a classic application expects.

The best I can think to do is to modify the standard and JNode-specific Stream wrapper classes to test the stream being wrapped to see if it is a proxy ... and wrap the proxied stream instead of the proxy. This would entail modifying OpenJDK code.

As for your proposed solution, I'm sorry but I cannot figure out what you are saying.

#14

After more thinking it seems to me that the problem cannot be fixed with a proxy stream. The only solution would be to override the access to the System.out field which doesn't seem to be worth the effort because we have a solution for that already in the form of isolates. So the real problem is that running jetty and other similar programs as a proclet doesn't make sense because the proclet support was not created for running all kind of Java code. It was only created for running proclets which must obey to certain constraints (these remain to be clearly specified and documented). Therefor having a shell with only the proclet invoker in operation is the root of the problem. We need a shell which will select and use the right invoker depending on the type of the code to be run.

So far I know of three kinds of executables:
- simple shell commands (nearly shell built-ins) for which the simple threaded invoker is ok
- proclets: more complex commands which should be run with the proclet invoker
- other code which should be run with the isolate invoker

For the 1st and 2nd case we can use either annotations or command descriptor attributes or both (or other mechanisms) to specify the type of the code: built-in or proclet which can be inspected by the shell for choosing the suitable invoker. For the code which doesn't have such attributes (it's neither a built-in nor a proclet) the shell should use the isolate invoker by default.
This solution also involves the fixing of the error stream for the isolate invoker (this would also help the case of jetty on the short term).

What do you think about this and how much time do you think we need to get it done?

#15

Here is another idea ... which might work.

The purpose of a proclet proxy stream is to 'resolve' to a (potentially) different stream in each proclet context. This is currently done by defining an inheritable thread local to hold each proclet's streams vector. An IO operation on the proxy stream 'resolves' to the proclet specific stream and dispatches the operation to that stream. A call to System.setIn/Out/Err causes the current proclet's streams vector to be updated.

Here is the idea. Instead of using thread locals to hold the streams vectors, hold them in central data structure; e.g. a HashMap keyed by a unique proclet id. Then, when setIn/Out/Err is called, it does the following:

  1. Clone the HashMap and its values.
  2. Use the supplied Stream to update the current proclet's entry in the clone HashMap.
  3. Create a new ProcletProxyStream wrapping the clone HashMap.
  4. Use reflection to atomically update the System.in/out/err 'variable'.

This should have the following effect.

  • Any application that looks at System.in/out/err will see proclet proxy streams reflecting the most recent setIn/Out/Err calls for the proclet.
  • If an application saves a proclet proxy (e.g. by wrapping it in another stream), the saved proxy will not be affected by subsequent setIn/Out/Err calls.

The net result is that setOut(new PrintStream(System.out)) etc should not produce a loop in the stream chain. If someone can verify my logic, I'll have a go at implementing this.

#17

You might think of other solutions to work around the problem but that will not solve the real problem which is that one kind of invoker is not suitable for everything. The proclet is only able to give a special handling to the System.out/in/err fields but how about the other static fields of an application where collisions might appear. Proclets were an idea for avoiding the overhead of isolates for relatively simple programs that are under our control in some way. For the rest we need isolates and as soon as a new isolate is started the enabled proclet mechanism in the parent isolate should not affect it. So I think we better start thinking about refactoring the shell in order to support the various kind of invokers. In fact the invoker should be an internal implementation detail of the shell completely invisible to users. So letting the user to set an invoker via a system property was ok in an experimental stage but should go away as the shell matures. I think it's time to go to the next level.

#18

Unfortunately, the proclet stream proxy mechanism (PSPM) is global in nature. Once the PSPM has been started in an isolate, anything in the isolate that refers to System.in/out/err will end up using the PSPM proxies, whether it is launched via ProcletInvoker, ThreadInvoker, DefaultInvoker or via new Thread(). I think this means that your idea above doesn't help.

By the way, the shell does not need refactoring to handle different invokers. It handles them already.

Anyway, I think that the ideal solution is to get isolates working well enough that we can run (almost) every shell, command and application in its own isolate. That should have other benefits, such as allowing applications to be stopped and killed.

(If all else fails, a "really clever" alternative to PSPM would be to modify the native compiler to compile a bytecode that fetches the System.in static (etc) into a Java method call, and have that method return a proclet context-specific stream.)

#19

I have thrown together a patch that implements the idea described in #15 with some minor tweaks. It seems to work for "normal" programs, so please try it out and see if it fixes the Jetty and DaCapo problems.

AttachmentSize
procletPatch.txt16.95 KB

#20

I've tried it and now jetty throws an NPE on startup at: AbstrcactProxyPrintStream.java:106.
Jetty is part of JNode, so you can test it too.

#21

Status:active» fixed

I've checked in the definitive fix, and the test application I used to verify its behavior. I still couldn't get Jetty to run, but I don't think the remaining problems are related to this issue. Marking as fixed.

#23

Status:fixed» closed

Closing by hand