gRPC websocket streams should close sanely

[niloc132]

Websocket-based gRPC-web streams were not correctly notifying gRPC internals when the stream was finished, this patch removes a poor attempt at that and correctly ends the stream.

This diff also adds java.util.logging support, forwarding it to slf4j, and sending slf4j's log level configuration back into java.util.logging.

Finally, this patch simplifies directly running main()s in the server project, so that they share configuration with the actual declare application.

Fixes #1745

[niloc132]

It seems like there should be a saner place to put this, but we don't have a centralized place to set up logging for all processes, so I started just with the two server mains. The initialize method in the log factory provider doesn't work, as that won't be called in this case until after the error in this bug has happened.

I'm not totally convinced we should have a single place for this, since the various client CLI mains seem to prefer to send to logback, so we wouldn't want to necessarily do the same log setup that exists in this class anyway?

Open to feedback and advice.

[devinrsmith]

I'm happy to be explicit here for now, generalize later if necessary.

[devinrsmith]

As formulated, this likely overwrites what we've set in java-open-nio.

I think jvmArgs += applicationDefaultJvmArgs would be better?

[niloc132]

I'll confirm this when i'm back, but applicationDefaultJvmArgs was already itself picking up that addopens, so I don't think it should need further processing?

(snippets edited to add artificial line breaks to wrap text)

> Task :server-jetty:JettyMain.main()
Caching disabled for task ':server-jetty:JettyMain.main()' because:
  Build cache is disabled
Task ':server-jetty:JettyMain.main()' is not up-to-date because:
  Task has not declared any outputs despite executing actions.
Starting process 'command '/usr/lib/jvm/java-11-openjdk/bin/java''. Working directory: /home/colin/workspace/illumon/core.new Command: /usr/lib/jvm/java-11-openjdk/bin/java 
    --add-opens java.base/java.nio=ALL-UNNAMED 
    -server 
    -XX:+UseG1GC 
    -XX:MaxGCPauseMillis=100 
    -XX:+UseStringDeduplication 
    -XshowSettings:vm 
    -XX:InitialRAMPercentage=25.0 
    -XX:MinRAMPercentage=70.0 
    -XX:MaxRAMPercentage=80.0 
    -Dfile.encoding=UTF-8 
    -Duser.country=US 
    -Duser.language=en 
    -Duser...

ACtually it looks like there is some other interaction going on here, like the :run itself is a JavaExec with defaults that come from applicationDefaultJvmArgs appended to the task's own args, so they get doubled:

> Task :server-jetty:run
Caching disabled for task ':server-jetty:run' because:
  Build cache is disabled
Task ':server-jetty:run' is not up-to-date because:
  Task has not declared any outputs despite executing actions.
Starting process 'command '/usr/lib/jvm/java-11-openjdk/bin/java''. Working directory: /home/colin/workspace/illumon/core.new/server/jetty Command: /usr/lib/jvm/java-11-openjdk/bin/java 
    -Ddeephaven.console.type=groovy 
    -server 
    -XX:+UseG1GC 
    -XX:MaxGCPauseMillis=100 
    -XX:+UseStringDeduplication 
    -XshowSettings:vm 
    -XX:InitialRAMPercentage=25.0 
    -XX:MinRAMPercentage=70.0 
    -XX:MaxRAMPercentage=80.0 
    --add-opens java.base/java.nio=ALL-UNNAMED 
    -server 
    -XX:+UseG1GC 
    -XX:MaxGCPauseMillis=100 
    -XX:+UseStringDeduplication 
    -XshowSettings:vm 
    -XX:InitialRAMPercentage=25.0 
    -XX:MinRAMPercentage=70.0 
    -XX:MaxRAMPercentage=80.0 
    -Dfile.encoding=UTF-8 
    -Duser.country=US 
    -Duser.language=en 
    -Duser...

[niloc132]

Some notes on this:

• the jvmArgs(...) method strictly appends. To replace, use setJvmArgs(...), which clears the extra jvm args and then reassigns, or setAllJvmArgs(...) which clears other properties as well. See org.gradle.process.internal.JvmOptions
• It also parses the incoming flags, and passes some off to other fields/methods, like assertionsEnabled, systemProperty(), etc.
• This means that interacting with "jvmArgs" in a JavaExec closure behaves differently based on using it like a method or a field. I don't love groovy. However, it does mean that this code is the correct way to assign these missing properties for IJ run configs, if I had written jvmArgs = applicationDefaultJvmArgs, it would have done as you described, mostly.
• Repeating this for clarity: Note that the setJvmArgs method does not unassign Xmx or system props, but you can at least re-assign them. Doesn't apply here, but it could bite one day.

Okay, knowing that, we want to set three options for three things:

1. script for dists
2. JavaExec for the :run task
3. other arbitrary JavaExecs

A tasks.withType(JavaExec) lets us set 2 and 3. A applicationDefaultJvmArgs sets 1 and 2. tasks.withType(CreateStartScripts) only sets 1. So applying tasks.withType for both JavaExec and CreateStartScripts seems like the simplest option, and omit applicationDefaultJvmArgs.

[niloc132]

Tested with running each of these three ways, and invoking jps -v | grep JettyMain to check for missing/dup args.

[devinrsmith]

Good to know, thanks for the investigation.