Mill output from server is truncated

[ajrnz]

There definitely seems to be some sort of miscommunication from the server to the client regarding termination. For example if you write a program:

object MillTest extends App {
  (1 to 100000).foreach{ i =>
    println(i)
  }
}

in a mill-test project and run

mill mill-test.run > result.txt

the file result.txt is truncated. The same happens if you run it to the console. Running with -i does not have the problem.

[If you think this is unrelated I am happy to open it as its own issue]

Originally posted by @ajrnz in https://github.com/com-lihaoyi/mill/issues/1711#issuecomment-1071354558

[ajrnz]

I'm pretty sure that is the cause of a number of issues with the 0.10 versions. I've spent a few hours on it and am making progress

[lefou]

@ajrnz If you think you're stuck, you could also have a look at the relevant PRs. I remember there were one or two, where we touched the client server communication. Probably that one which claimed to "fix" some communication issues. If I find it, I'll add a link.

Your work is very appreciated!

Edit:

Here are some PRs which touched the client server communication

• 1621

• 1644

• 1645

[ajrnz]

@lefou I'd like to add a test for this if possible. Where (and how) would be the best place to try and add it? It requires mill to be used in client / server mode.

[lefou]

I think the best place is the integration module. I'm currently in the process of refactoring this very module, but If you create a new test class/object, our work should not interfere much. Also, I'm not sure if mills ScriptTestSuite works with client-server mode out of the box, but you never know.

You could also have a look at how mill-integrationtest plugin is running mill tests, but even this one currently enforces --no-server mode.

[ajrnz]

Ah ok. It doesn't look like anything does it. It might be more hassle than it is worth.

[lefou]

I think it's definitely worth it, but also takes some efforts. That's probably the reason, it's not there, yet.

[ajrnz]

Phew! This took a few hours to hunt down. I had to write interceptors for all the streams to locate the issue. The problem is due to data being present in the unix domain pipe and not being read by the client before the socket is closed.

In general what happens is this:

• the server runs the job (eg the small program above)
• the server writes all the stdout/stderr data to the pipe
• it then signals to the client that it has finished by releasing the lock
• the client sees that the lock has been released and closes the socket
• unfortunately the client doesn't know it hasn't read all the data yet - hence the truncation

Neither the server nor the client have a way of knowing that the pipe has completely read (the client pumper is waiting on a JNI based read() which is not terminated with the socket closure). The 'correct' way to solve this would be to signal the end of the stream in the pipe itself but this would require significant refactoring.

A more pragmatic and localised way to solve this problem is to add a small delay in the client to catch all the data. I've done this by adding a mechanism which waits for a period of no data lasting 50ms.

On my Mac with data in the buffer, the wait is typically about 35ms for the data and then 50ms of silence, so ~85ms, which is pretty negligible. It seems pretty robust.

See PR #1828

[ajrnz]

BTW: I am no longer convinced that this is the cause of other issues I am seeing - there are various related to -w which manifest themselves in different ways eg #1711. I'll try to take a look at it if I get some more time.

[lefou]

@ajrnz I reorganized the test suite. The best place to add your test would be integration.forked. The IntegrationTestSuite, which extends ScriptTestSuite currently hardcodes the -i option (https://github.com/com-lihaoyi/mill/blob/e7d9027f54170e69a2f76764e5669d3ced442e00/main/test/src/util/ScriptTestSuite.scala#L94), but I think you should try to experimental remove it. It should work, but will leave lots of running mill servers behind. Those can be cleaned up with mill clean for now; later we can use and improve the shutdown command, and call it automatically. If it works, we could make it configurable via a flag and enhance the forked test suite by a new with-server dimension. Just ping me on Gitter, if you want to chat.

[lefou]

See also my PR https://github.com/com-lihaoyi/mill/pull/1835, which adds client-server tests. It works on my machine (Linux), but no Idea about Windows. 🤞‍

[lefou]

It would be interesting to check, if the new forked-server tests are able to detect truncated output...