EvaluateExpressionRaw silently drops ctx cancellation as (nil, nil), and can permanently deadlock the evaluator manager

[ishaagg29]

Summary

When the caller's context is cancelled/times out while an Evaluate request is in flight, EvaluateExpressionRaw returns (nil, nil) instead of the context error. Combined with an unbuffered response channel that is never cleaned up from pendingRequests, this can permanently deadlock the shared evaluatorManager.listen() goroutine — breaking all subsequent evaluations on that evaluator until the process is restarted.

Where

https://github.com/apple/pkl-go/blob/v0.10.0/pkl/evaluator.go#L105-L132

func (e *evaluator) EvaluateExpressionRaw(ctx context.Context, source *ModuleSource, expr string) ([]byte, error) {
      ...
      ch := make(chan *msgapi.EvaluateResponse)
      e.pendingRequests.Store(requestId, ch)
      ...
      select {
      case <-ctx.Done():
              return nil, nil   // (1) swallows ctx.Err(); (2) never removes requestId from pendingRequests
      case err := <-interrupted:
              return nil, err   // same missing cleanup
      case resp := <-ch:
              ...
      }
}

Impact 1 — misleading errors downstream

EvaluateExpression (evaluator.go:97-103) treats (nil, nil) as success and calls Unmarshal(nil, out), which builds msgpack.NewDecoder(bytes.NewReader(nil)) and returns io.EOF on the first read. Callers that expected a timeout/cancellation error instead see a generic io.EOF, indistinguishable from a real decode failure, so they cannot reasonably distinguish "operation timed out" from "malformed
response".

Impact 2 — permanent deadlock

handleEvaluateResponse (evaluator.go:146-156) does:

  ch <- resp   // unbuffered send
  close(ch)
  e.pendingRequests.Delete(resp.RequestId)

If the caller already returned via the ctx.Done() branch above, nobody is receiving on ch. Since handleEvaluateResponse is invoked synchronously from the single evaluatorManager.listen() goroutine (evaluator_manager_exec.go:60, evaluator_manager.go:185-236), that goroutine is the only reader of all incoming messages from the pkl subprocess for that manager. A single late response after a client-side timeout permanently blocks that goroutine — and therefore every other in-flight or future evaluation on the same evaluator — until the process holding it is restarted.

The same unbuffered-channel-without-cleanup pattern also exists in evaluator creation: evaluator_manager.go:111-131.

Suggested fix

• Return ctx.Err() (and the interrupted error) instead of nil, nil.
• Delete the pendingRequests entry on the cancellation/interrupt path.
• Make the response channel buffered (capacity 1) so a late/racing send from handleEvaluateResponse cannot block the shared dispatch goroutine even if cleanup and the incoming message race each other.

PR : #262