Repository: apple/pkl
File: pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java (Java)

Code before:
   *
   * <p>Blocks until the underlying transport is closed.
   */
  private void runTransport() {
    try {
      transport.start(
          (msg) -> {
            throw new ProtocolException("Unexpected incoming one-way message: " + msg);
          },
          (msg) -> {
            throw new ProtocolException("Unexpected incoming request message: " + msg);
          });
    } catch (ProtocolException | IOException e) {
      throw new RuntimeException(e);
    }
  }

  @Override
  public void close() {
    synchronized (lock) {
      closed = true;
      if (process == null || !process.isAlive()) {
        return;
      }

      try {
        if (transport != null) {
          transport.send(new CloseExternalProcess());
          transport.close();
        }

        // forcefully stop the process after the timeout
        // note that both transport.close() and process.destroy() are safe to call multiple times
        new Timer()
            .schedule(
                new TimerTask() {
                  @Override
                  public void run() {
                    if (process != null) {
                      transport.close();
                      process.destroyForcibly();
                    }
                  }
                },
                CLOSE_TIMEOUT.inWholeMillis());

        // block on process exit
        process.onExit().get();
      } catch (Exception e) {
        transport.close();
        process.destroyForcibly();

Review comment (suggestion):
A timer shouldn't be necessary here. I'd expect something like the following to work.
The `!= null` checks guard against a process/transport that wasn't initialized.
`CLOSE_TIMEOUT` was changed to `java.time.Duration`.
```
  @Override
  public void close() {
    synchronized (lock) {
      if (closed) return;
      closed = true;
      try {
        if (transport != null && process != null && process.isAlive()) {
          transport.send(new CloseExternalProcess());
          process.waitFor(CLOSE_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS);
        }
      } catch (Exception ignored) {
      } finally {
        if (process != null) {
          // no-op unless process is alive
          process.destroyForcibly();
        }
        if (transport != null) {
          transport.close();
        }
      }
    }
  }
```

Code after:
   * <p>Blocks until the underlying transport is closed.
   */
  private void runTransport() {
    try {
      transport.start(
          (msg) -> {
            throw new ProtocolException("Unexpected incoming one-way message: " + msg);
          },
          (msg) -> {
            throw new ProtocolException("Unexpected incoming request message: " + msg);
          });
    } catch (ProtocolException | IOException e) {
      throw new RuntimeException(e);
    }
  }

  @Override
  public void close() {
    synchronized (lock) {
      if (closed) return;
      closed = true;

      try {
        if (transport != null && process != null && process.isAlive()) {
          transport.send(new CloseExternalProcess());
          process.waitFor(CLOSE_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS);
        }
      } catch (Exception ignored) {
      } finally {
        if (process != null) {
          // no-op unless process is alive
          process.destroyForcibly();
        }
        if (transport != null) {
          transport.close();
        }
      }
    }
  }

  @Override
  public ExternalModuleResolver.@Nullable Spec getModuleReaderSpec(String uriScheme)
      throws IOException {
    return MessageTransports.resolveFuture(
        initializeModuleReaderResponses.computeIfAbsent(
            uriScheme,
            (scheme) -> {
              var future = new CompletableFuture<ExternalModuleResolver.@Nullable Spec>();
              var request =
                  new InitializeModuleReaderRequest(requestIdGenerator.nextLong(), scheme);
              try {