workflows/wasm/host: prevent limiter and Wasmtime resource leaks

[cfal]

fixes resource leaks in the WASM host by explicitly closing module-owned limiters and Wasmtime resources.

confidential-workflows creates a new module per production execution without injecting limiters. the previous fallback limiters started 27 goroutines per module and were never closed.

this change:

• replaces constant fallback bounds with static limiters
• closes owned resources on constructor failure and module shutdown
• keeps fallback limiters module-local so async eviction cleanup can't affect a replacement module
• closes temporary modules, linkers, and Wasmtime resources deterministically

[github-actions]

👋 cfal, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

[github-actions]

📊 API Diff Results

No changes detected for module github.com/smartcontractkit/chainlink-common

View full report

[cfal]

removed because settings.Size only wrapped the raw bound with a parser and unit:

chainlink-common/pkg/settings/settings.go

Lines 160 to 163 in 6b01cb8

	func Size(defaultValue config.Size) Setting[config.Size] {
	s := NewSetting(defaultValue, config.ParseByte)
	s.Unit = "By"
	return s

this call site constructed a Factory with only Logger, so Settings and Meter were nil:

chainlink-common/pkg/workflows/wasm/host/module.go

Lines 287 to 296 in 6b01cb8

	lf := limits.Factory{Logger: modCfg.Logger}
	if modCfg.EnableUserMetricsLimiter == nil {
	modCfg.EnableUserMetricsLimiter = limits.NewGateLimiter(false)
	}
	if modCfg.MaxUserMetricPayloadLimiter == nil {
	limit := settings.Size(config.Size(modCfg.MaxUserMetricPayloadBytes))
	var err error
	modCfg.MaxUserMetricPayloadLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)

GetOrDefault returns DefaultValue immediately when Settings is nil:

chainlink-common/pkg/settings/settings.go

Lines 216 to 229 in 6b01cb8

	// GetOrDefault gets the setting from the Getter for the given Scope, or returns the default value with an error.
	func (s *Setting[T]) GetOrDefault(ctx context.Context, g Getter) (value T, err error) {
	if g == nil {
	return s.DefaultValue, nil
	}
	str, err := g.GetScoped(ctx, s.Scope, s.Key)
	if err != nil || str == "" {
	return s.DefaultValue, err
	}
	value, err = s.Parse(str)
	if err != nil {
	return s.DefaultValue, err
	}
	return value, nil

despite that, MakeUpperBoundLimiter started a global updater:

chainlink-common/pkg/settings/limits/bound.go

Lines 65 to 124 in 6b01cb8

	func newBoundLimiter[N Number](f Factory, bound settings.SettingSpec[N], isLowerBound bool) (BoundLimiter[N], error) {
	b := &boundLimiter[N]{
	updater: newUpdater[N](nil, func(ctx context.Context) (N, error) {
	return bound.GetOrDefault(ctx, f.Settings)
	}, nil),
	key: bound.GetKey(),
	scope: bound.GetScope(),
	isLowerBound: isLowerBound,
	}
	b.recordLimit = func(ctx context.Context, n N) { b.recordBound(ctx, n) }
	if f.Meter != nil {
	if b.key == "" {
	return nil, errors.New("metrics require Key to be set")
	}
	newGauge, newHist := metricConstructors[N](f.Meter, bound.GetUnit())
	key := bound.GetKey()
	limitGauge, err := newGauge("bound." + key + ".limit")
	if err != nil {
	return nil, err
	}
	b.recordBound = func(ctx context.Context, value N, options ...metric.RecordOption) {
	limitGauge.Record(ctx, value, options...)
	}
	usageHist, err := newHist("bound." + key + ".usage")
	if err != nil {
	return nil, err
	}
	b.recordUsage = func(ctx context.Context, value N, options ...metric.RecordOption) {
	usageHist.Record(ctx, value, options...)
	}
	deniedHist, err := newHist("bound." + key + ".denied")
	if err != nil {
	return nil, err
	}
	b.recordDenied = func(ctx context.Context, value N, options ...metric.RecordOption) {
	deniedHist.Record(ctx, value, options...)
	}
	} else {
	b.recordBound = func(ctx context.Context, value N, options ...metric.RecordOption) {}
	b.recordUsage = func(ctx context.Context, value N, options ...metric.RecordOption) {}
	b.recordDenied = func(ctx context.Context, value N, options ...metric.RecordOption) {}
	}
	if f.Logger != nil {
	b.lggr = logger.Sugared(f.Logger).Named("BoundLimiter").With("key", bound.GetKey())
	}
	if f.Settings != nil {
	if r, ok := f.Settings.(settings.Registry); ok {
	b.subFn = func(ctx context.Context) (<-chan settings.Update[N], func()) {
	return bound.Subscribe(ctx, r)
	}
	}
	}
	if bound.GetScope() == settings.ScopeGlobal {
	go b.updateLoop(context.Background())
	}

each updater also started cancellation and ticker goroutines:

chainlink-common/pkg/settings/limits/updater.go

Lines 68 to 84 in 6b01cb8

	func (u *updater[N]) updateLoop(ctx context.Context) {
	defer close(u.done)
	ctx, cancel := u.stopCh.Ctx(context.WithoutCancel(ctx))
	defer cancel()
	var updates <-chan settings.Update[N]
	var cancelSub func()
	var c <-chan time.Time
	if u.subFn != nil {
	updates, cancelSub = u.subFn(ctx)
	defer func() { cancelSub() }() // extra func wrapper is required to ensure we get the final cancelSub value
	// opt: poll now to initialize
	} else {
	t := services.TickerConfig{}.NewTicker(pollPeriod)
	defer t.Stop()
	c = t.C
	}

chainlink-common/pkg/services/stop_!race.go

Lines 9 to 17 in 6b01cb8

	func (s StopRChan) CtxCancel(ctx context.Context, cancel context.CancelFunc) (context.Context, context.CancelFunc) {
	go func() {
	select {
	case <-s:
	cancel()
	case <-ctx.Done():
	}
	}()
	return ctx, cancel

chainlink-common/pkg/timeutil/ticker.go

Lines 14 to 20 in 6b01cb8

	// NewTicker returns a started Ticker which calls nextDur for each period.
	// Ticker.Stop should be called to prevent goroutine leaks.
	func NewTicker(nextDur func() time.Duration) *Ticker {
	c := make(chan time.Time) // unbuffered so we block and delay if not being handled
	t := Ticker{C: c, stop: make(chan struct{}), reset: make(chan struct{})}
	go t.run(c, nextDur)
	return &t

so these nine constant defaults created 27 goroutines per module to repeatedly resolve the same value. NewUpperBoundLimiter enforces the same bound without that unused update machinery.

callers that need dynamic settings already inject configured limiters through ModuleConfig:

https://github.com/smartcontractkit/chainlink/blob/f05ecfe30b60d0be98aabbb1052657ef352b74d8/core/services/workflows/syncer/v2/handler.go#L836-L848

[jmank88]

callers that need dynamic settings already inject configured limiters through ModuleConfig:

IIRC we expect all production callers to do this. This other case was only meant to be a non-production fallback. Do we have callers in production that are not injecting limiters?

[cfal]

confidential-workflows creates a module per production execution without injecting any limiters:

https://github.com/smartcontractkit/chainlink-confidential-compute/blob/19cbd1cb7c4440fee21caddee229a8dba3a14f28/enclave/apps/confidential-workflows/app/wasm.go#L28-L44

WasmFileSpecFactory is another production path that supplies only Logger:

https://github.com/smartcontractkit/chainlink/blob/be67b7b77be4d515be111ddc935696b58edcec53/core/services/job/wasm_file_spec_factory.go#L24-L36

the standard workflow runtime also omits MaxSubscriptionsLimiter.

[jmank88]

Should we set them from confidential compute? Otherwise we cannot reconfigure and we don't get metrics.

The other one seems less important, given that it already has a null logger?

[cfal]

used to be written into the caller-owned ModuleConfig:

chainlink-common/pkg/workflows/wasm/host/module.go

Lines 293 to 368 in 6b01cb8

	if modCfg.MaxUserMetricPayloadLimiter == nil {
	limit := settings.Size(config.Size(modCfg.MaxUserMetricPayloadBytes))
	var err error
	modCfg.MaxUserMetricPayloadLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make metric payload size limiter: %w", err)
	}
	}
	if modCfg.MaxUserMetricNameLengthLimiter == nil {
	limit := settings.Int(int(modCfg.MaxUserMetricNameLength))
	var err error
	modCfg.MaxUserMetricNameLengthLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make metric name length limiter: %w", err)
	}
	}
	if modCfg.MaxUserMetricLabelsPerMetricLimiter == nil {
	limit := settings.Int(int(modCfg.MaxUserMetricLabelsPerMetric))
	var err error
	modCfg.MaxUserMetricLabelsPerMetricLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make labels per metric limiter: %w", err)
	}
	}
	if modCfg.MaxUserMetricLabelValueLengthLimiter == nil {
	limit := settings.Int(int(modCfg.MaxUserMetricLabelValueLength))
	var err error
	modCfg.MaxUserMetricLabelValueLengthLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make label value length limiter: %w", err)
	}
	}
	if modCfg.MemoryLimiter == nil {
	// Take the max of the min and the configured max memory mbs.
	// We do this because Go requires a minimum of 16 megabytes to run,
	// and local testing has shown that with less than the min, some
	// binaries may error sporadically.
	modCfg.MaxMemoryMBs = uint64(math.Max(float64(modCfg.MinMemoryMBs), float64(modCfg.MaxMemoryMBs)))
	limit := settings.Size(config.Size(modCfg.MaxMemoryMBs) * config.MByte)
	var err error
	modCfg.MemoryLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make memory limiter: %w", err)
	}
	}
	if modCfg.MaxCompressedBinaryLimiter == nil {
	limit := settings.Size(config.Size(modCfg.MaxCompressedBinarySize))
	var err error
	modCfg.MaxCompressedBinaryLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make compressed binary size limiter: %w", err)
	}
	}
	if modCfg.MaxDecompressedBinaryLimiter == nil {
	limit := settings.Size(config.Size(modCfg.MaxDecompressedBinarySize))
	var err error
	modCfg.MaxDecompressedBinaryLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make decompressed binary size limiter: %w", err)
	}
	}
	if modCfg.MaxResponseSizeLimiter == nil {
	limit := settings.Size(config.Size(modCfg.MaxResponseSizeBytes))
	var err error
	modCfg.MaxResponseSizeLimiter, err = limits.MakeUpperBoundLimiter(lf, limit)
	if err != nil {
	return nil, fmt.Errorf("failed to make response size limiter: %w", err)
	}
	}
	if modCfg.MaxSubscriptionsLimiter == nil {
	var err error
	modCfg.MaxSubscriptionsLimiter, err = limits.MakeUpperBoundLimiter(lf, cresettings.Default.WASMPollOneoffSubscriptionLimit)
	if err != nil {
	return nil, fmt.Errorf("failed to make poll_oneoff subscription limiter: %w", err)
	}
	}

that isn’t safe as EvictableModule retains the same config pointer, the old module closes asynchronously, and a replacement can be constructed while that cleanup is still queued:

https://github.com/smartcontractkit/chainlink/blob/f05ecfe30b60d0be98aabbb1052657ef352b74d8/core/services/workflows/syncer/v2/handler.go#L914

https://github.com/smartcontractkit/chainlink/blob/f05ecfe30b60d0be98aabbb1052657ef352b74d8/core/services/workflows/syncer/v2/evictable_module.go#L131-L150

https://github.com/smartcontractkit/chainlink/blob/f05ecfe30b60d0be98aabbb1052657ef352b74d8/core/services/workflows/syncer/v2/evictable_module.go#L291-L336

keeping defaults per-module means the old module can only close its own resources.

[jmank88]

Can you please add a description? I'm having trouble understanding the goal of this PR.

[jmank88]

Why switch from the *Make(Factory,* variants? The only effect should be losing logging, which is especially important while we still have "fail open" cases which only log errors rather than return them.

[cfal]

there is no effective logging lost. Factory.Settings is nil, so GetOrDefault always returns the default with a nil error. the fail-open log path only applies to missing tenants on non-global settings, while these defaults are all global. bound violations weren't logged before either.

so NewUpperBoundLimiter preserves the observable behavior; it only removes the unused updater machinery.

[jmank88]

I don't like the idea of depending on implementation details that could change but if this is only for the non-production cases then that is probably fine. Can we update the injection from confidential compute?
Otherwise, if we are stuck with using these for production, then something seems misaligned, because we should want overrides and metrics in that case, no?

replied to comments