fix(plugins): prevent concurrent WASM compilation race condition (#4253)

* fix: eliminate race condition in plugin system

Added compilation waiting mechanism to prevent WASM plugins from being instantiated
before their background compilation completes. This fixes the intermittent error
'source module must be compiled before instantiation' that occurred when tests
or plugin usage happened before asynchronous compilation finished.

Changes include:
- Added manager reference to wasmBasePlugin for compilation synchronization
- Modified all plugin adapter constructors to accept manager parameter
- Updated getInstance() to wait for compilation before loading instances
- Fixed runtime test to handle manually created plugins appropriately

The race condition was caused by plugins trying to compile WASM modules
synchronously during Load() calls while background compilation was still
in progress. This change ensures proper coordination between the compilation
and instantiation phases.

* fix: add plugin-clean target to Makefile for easier plugin cleanup

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: reorder plugin constructor parameters and add nil safety

Moved manager parameter to third position in pluginConstructor signature for\nbetter parameter ordering consistency.\n\nAlso added nil check for adapter creation to prevent registration of failed\nplugin adapters, which could lead to nil-pointer dereferences. Plugin\ncreation failures are now logged with context and gracefully skipped.\n\nChanges:\n- Reordered pluginConstructor parameters: manager moved before runtime\n- Updated all 4 adapter constructor signatures to match new order\n- Added nil safety check in registerPlugin to skip failed adapters\n- Updated runtime test to use new parameter order\n\nThis improves both code consistency and runtime safety by preventing\nnil adapters from being registered in the plugin manager.

* fix: prevent concurrent WASM compilation race condition

* refactor: remove unnecessary manager parameter from plugin constructors

* fix: update parameter name in newWasmSchedulerCallback for consistency

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan Quintão 2025-06-23 11:51:30 -04:00 committed by GitHub
parent cfa1d7fa81
commit 1bec99a2f8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 29 additions and 4 deletions

View File

@ -230,6 +230,11 @@ plugin-examples: check_go_env ##@Development Build all example plugins
$(MAKE) -C plugins/examples clean all
.PHONY: plugin-examples
plugin-clean: check_go_env ##@Development Clean all plugins
$(MAKE) -C plugins/examples clean
$(MAKE) -C plugins/testdata clean
.PHONY: plugin-clean
plugin-tests: check_go_env ##@Development Build all test plugins
$(MAKE) -C plugins/testdata clean all
.PHONY: plugin-tests

View File

@ -9,16 +9,16 @@ import (
)
// newWasmSchedulerCallback creates a new adapter for a SchedulerCallback plugin
func newWasmSchedulerCallback(wasmPath, pluginName string, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin {
func newWasmSchedulerCallback(wasmPath, pluginID string, runtime api.WazeroNewRuntime, mc wazero.ModuleConfig) WasmPlugin {
loader, err := api.NewSchedulerCallbackPlugin(context.Background(), api.WazeroRuntime(runtime), api.WazeroModuleConfig(mc))
if err != nil {
log.Error("Error creating scheduler callback plugin", "plugin", pluginName, "path", wasmPath, err)
log.Error("Error creating scheduler callback plugin", "plugin", pluginID, "path", wasmPath, err)
return nil
}
return &wasmSchedulerCallback{
wasmBasePlugin: &wasmBasePlugin[api.SchedulerCallback, *api.SchedulerCallbackPlugin]{
wasmPath: wasmPath,
id: pluginName,
id: pluginID,
capability: CapabilitySchedulerCallback,
loader: loader,
loadFunc: func(ctx context.Context, l *api.SchedulerCallbackPlugin, path string) (api.SchedulerCallback, error) {

View File

@ -161,6 +161,10 @@ func (m *Manager) registerPlugin(pluginID, pluginDir, wasmPath string, manifest
continue
}
adapter := constructor(wasmPath, pluginID, customRuntime, mc)
if adapter == nil {
log.Error("Failed to create plugin adapter", "plugin", pluginID, "capability", capabilityStr, "path", wasmPath)
continue
}
m.adapters[pluginID+"_"+capabilityStr] = adapter
}

View File

@ -520,6 +520,9 @@ type cachingRuntime struct {
// poolInitOnce ensures the pool is initialized only once
poolInitOnce sync.Once
// compilationMu ensures only one compilation happens at a time per runtime
compilationMu sync.Mutex
}
func newCachingRuntime(runtime wazero.Runtime, pluginID string) *cachingRuntime {
@ -580,7 +583,7 @@ func (r *cachingRuntime) setCachedModule(module wazero.CompiledModule, wasmBytes
func (r *cachingRuntime) CompileModule(ctx context.Context, wasmBytes []byte) (wazero.CompiledModule, error) {
incomingHash := md5.Sum(wasmBytes)
// Try to get from cache
// Try to get from cache first (without lock for performance)
if cached := r.cachedModule.Load(); cached != nil {
if module := cached.get(incomingHash); module != nil {
log.Trace(ctx, "cachingRuntime: using cached compiled module", "plugin", r.pluginID)
@ -588,6 +591,18 @@ func (r *cachingRuntime) CompileModule(ctx context.Context, wasmBytes []byte) (w
}
}
// Synchronize compilation to prevent concurrent compilation issues
r.compilationMu.Lock()
defer r.compilationMu.Unlock()
// Double-check cache after acquiring lock (another goroutine might have compiled it)
if cached := r.cachedModule.Load(); cached != nil {
if module := cached.get(incomingHash); module != nil {
log.Trace(ctx, "cachingRuntime: using cached compiled module (after lock)", "plugin", r.pluginID)
return module, nil
}
}
// Fall back to normal compilation for different bytes
log.Trace(ctx, "cachingRuntime: hash doesn't match cache, compiling normally", "plugin", r.pluginID)
module, err := r.Runtime.CompileModule(ctx, wasmBytes)

View File

@ -39,6 +39,7 @@ func (w *wasmBasePlugin[S, P]) getInstance(ctx context.Context, methodName strin
start := time.Now()
// Add context metadata for tracing
ctx = log.NewContext(ctx, "capability", w.serviceName(), "method", methodName)
inst, err := w.loadFunc(ctx, w.loader, w.wasmPath)
if err != nil {
var zero S