Isolates and pipelines not playing well together.

Project:JNode Shell
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:active
Description

There are multiple bugs with isolates and pipelines, i'll outline what i've found so far. What im looking for are suggestions on how this should be fixed. Some of the system involved im not sure how they are supposed to interact, so here goes.

Redirecting Interpreter

With the redirect interp, trying to pipeline two commands via isolate invoker causes IOUtils.findBaseStream to fail when using reflection to lookup the "out" field of the Writer to find the output stream. I'm guessing this is simply a permission issue, something that proclet has and isolate doesn't have set?

Bjorne Interpreter

If the invoker is set to isolate before the interpreter is set to bjorne, any attempt to run a command leads to an exception being thrown. I can't paste the original stack trace, but here's the basics.

CommandIOException: I/O not supported on CommandIOMarker
- CommandIOMarker.getPrintWriter(89)
- BaseCommandIO.getPrintWriter(113)
- CommandRunner.(88)
- AsyncCommandInvoker.setup(123)
- AsyncCommandInvoker.setup(98)
- AsyncCommandInvoker.invoke(86)
- CommandShell.invoke(647)
- BjorneInterpreter.executeCommand(343)
...

This in itself has nothing to do with pipelines.

If you set the interpreter to bjorne _then_ set the invoker to isolates, you can run simple commands, but pipelines cause the shell to lock-up. It wont recognize a ^C to kill the command, i can scroll the console buffer, and i can switch to the logger console. So jnode itself is not locked. Also my cpu usage on the host is not above normal, so perhaps it is simply deadlocked?

I'm not so much looking for fixes, as just some insights as to where to look to fix some of these issues. I'm going to try and debug the deadlock issue with isolate pipelines. But im not sure what to do about the redirecting interp issue, or why the CommandIOExceptions are being thrown.

#1

I've dug a bit into the deadlock issue. It happens when the right hand side of the pipeline goes to access its input stream. Any attempt to read() blocks.

#2

The "marker" objects are designed to be replaced by real stream objects before they reach application code. What you've found is a clear sign that something (most likely the IsolateCommandInvoker or related classes) is not properly implemented.

Frankly, I'm not surprised.

Update: ^C is currently disabled in the shell because the approach previously used for was unsafe. Based on what Peter has told me, the mechanism used to kill an isolate is also unsafe. Now I could simply wire ^C to these mechanisms, but I don't think it is a good idea to support ^C if it can potentially lead to a system lockup / file system corruption.

#3

Actually, I've been unable to reproducing this using "cat | cat" as a test, with any combination of "redirecting" / "bjorne" and "proclet" / "isolate". Can you please tell me what pipeline you are running to trigger the failure(s)?

#4

Marker Exception

propset -s jnode.invoker isolate
propset -s jnode.interpreter bjorne
vminfo | grep "TSI"

And actually you dont need to do a pipeline for this, this failed on any attempt to run a command.

Deadlock on read

propset -s jnode.interpreter bjorne
propset -s jnode.invoker isolate
vminfo | grep "TSI"

I know this is deadlocking on the read as i've debugged it. Down to my first attempt to readLine()

propset -s jnode.invoker isolate
vminfo | grep "TSI"

This results in a double stacktrace. One to the logger, and a normal stacktrace. The second stacktrace is simply a side effect of the first. Where the first is the result of a denial to read the protected "out" field of Writer when IOUtils.findBaseStream is called.

#5

Ah yes. I see. (I have a feeling I left off the "-s" when I tried to reproduce this before ... duh!)

Do you want to keep looking at this, or shall I?

#6

The issue with the the marker not being replaced might be easier for you to find since you know the code. It just seems completely odd to me that there is different behavior depending on the order in which the invoker/interpreter are set.

The deadlock im still poking around at, i'm just about done with the command refactoring and i'll jump back on to that asap. Although if you got to it first, i wouldn't hold it against you Eye-wink

I'll take my name off this for now, as i'm deep into something at the moment, and would like to see it resolved by whatever means so we can flush out the rest of the invoker code. From what i understand there are parts of this there were untested/incomplete, and i'd like to see it runnable and perhaps stable enough to set on by default in my config.

#7

Assigned to:cluster» Anonymous

#8

Assigned to:Anonymous» Stephen Crawley

Cool. I've already fixed the IOUtils bug. Assigning issue to me.

Re bizareness: Consider that the second 'propset' command will be run using the 'proclet' invoker in one case and the 'isolate' invoker in the other case. If something in the isolate invoker is not tidying up streams properly, that might cause a "marker" stream to leak to code that then tries to use it. Of course .... this is just a theory Smiling.

#9

Another fix committed. The isolate and proclets were not respecting the "internal" flag on propset. In the case of isolates, this meant that with:

    propset -s jnode.invoker isolate
    propset -s jnode.interpreter bjorne
    vminfo | grep "TSI"

... the second propset was actually changing shell properties in a different isolate to the shell where it will have no effect. This certainly explains why the order of the propset commands made a difference!!

This fix is also in the patch awaiting Levente's review. But we cannot wait, so I've committed the relevant part of the patch anyway.

Frankly, this illustrates how it is not a sustainable proposition for Levente to review committers' patches before they are committed. It is better to review changes after commit and make any required changes in a follow-up commit.

#10

Getting back to the original bugs reported:

  • The problem causing the security exception in IOUtils.findBaseStream has been fixed.
  • The problem with switching to isolates before switching to bjorne is no longer there. It was probably fixed by the changes that were needed to get the isolate properties and environment working. Any way, I cannot reproduce it, so I'm assuming it is fixed.
  • The isolate versus pipeline problem is still present. I can reproduce it with "echo hi | cat" with bjorne/isolate.

#11

Assigned to:Stephen Crawley» Anonymous

Deassigning this issue while I do some more Bjorne work.