fix #3825: memory leak of pluginData values

Author: evanw

CHANGELOG.md

@@ -32,6 +32,10 @@
 
     With this release, esbuild will attempt to print comments that come before case clauses in switch statements. This is similar to what esbuild already does for comments inside of certain types of expressions. Note that these types of comments are not printed if minification is enabled (specifically whitespace minification).
 
+* Fix a memory leak with `pluginData` ([#3825](https://github.com/evanw/esbuild/issues/3825))
+
+    With this release, the build context's internal `pluginData` cache will now be cleared when starting a new build. This should fix a leak of memory from plugins that return `pluginData` objects from `onResolve` and/or `onLoad` callbacks.
+
 ## 0.23.0
 
 **_This release deliberately contains backwards-incompatible changes._** To avoid automatically picking up releases like this, you should either be pinning the exact version of `esbuild` in your `package.json` file (recommended) or be using a version range syntax that only accepts patch upgrades such as `^0.22.0` or `~0.22.0`. See npm's documentation about [semver](https://docs.npmjs.com/cli/v6/using-npm/semver/) for more information.

cmd/esbuild/service.go

@@ -836,7 +836,6 @@ func (service *serviceType) convertPlugins(key int, jsPlugins interface{}, activ
 
 	var onResolveCallbacks []filteredCallback
 	var onLoadCallbacks []filteredCallback
-	hasOnStart := false
 	hasOnEnd := false
 
 	filteredCallbacks := func(pluginName string, kind string, items []interface{}) (result []filteredCallback, err error) {
@@ -860,10 +859,6 @@ func (service *serviceType) convertPlugins(key int, jsPlugins interface{}, activ
 		p := p.(map[string]interface{})
 		pluginName := p["name"].(string)
 
-		if p["onStart"].(bool) {
-			hasOnStart = true
-		}
-
 		if p["onEnd"].(bool) {
 			hasOnEnd = true
 		}
@@ -944,22 +939,20 @@ func (service *serviceType) convertPlugins(key int, jsPlugins interface{}, activ
 			}
 			activeBuild.mutex.Unlock()
 
-			// Only register "OnStart" if needed
-			if hasOnStart {
-				build.OnStart(func() (api.OnStartResult, error) {
-					response, ok := service.sendRequest(map[string]interface{}{
-						"command": "on-start",
-						"key":     key,
-					}).(map[string]interface{})
-					if !ok {
-						return api.OnStartResult{}, errors.New("The service was stopped")
-					}
-					return api.OnStartResult{
-						Errors:   decodeMessages(response["errors"].([]interface{})),
-						Warnings: decodeMessages(response["warnings"].([]interface{})),
-					}, nil
-				})
-			}
+			// Always register "OnStart" to clear "pluginData"
+			build.OnStart(func() (api.OnStartResult, error) {
+				response, ok := service.sendRequest(map[string]interface{}{
+					"command": "on-start",
+					"key":     key,
+				}).(map[string]interface{})
+				if !ok {
+					return api.OnStartResult{}, errors.New("The service was stopped")
+				}
+				return api.OnStartResult{
+					Errors:   decodeMessages(response["errors"].([]interface{})),
+					Warnings: decodeMessages(response["warnings"].([]interface{})),
+				}, nil
+			})
 
 			// Only register "OnResolve" if needed
 			if len(onResolveCallbacks) > 0 {

lib/shared/common.ts

@@ -1350,6 +1350,13 @@ let handlePlugins = async (
   }
 
   requestCallbacks['on-start'] = async (id, request: protocol.OnStartRequest) => {
+    // Reset the "pluginData" map before each new build to avoid a memory leak.
+    // This is done before each new build begins instead of after each build ends
+    // because I believe the current API doesn't restrict when you can call
+    // "resolve" and there may be some uses of it that call it around when the
+    // build ends, and we don't want to accidentally break those use cases.
+    details.clear()
+
     let response: protocol.OnStartResponse = { errors: [], warnings: [] }
     await Promise.all(onStartCallbacks.map(async ({ name, callback, note }) => {
       try {
@@ -1548,6 +1555,7 @@ let handlePlugins = async (
 // even if the JavaScript objects aren't serializable. And we also avoid
 // the overhead of serializing large JavaScript objects.
 interface ObjectStash {
+  clear(): void
   load(id: number): any
   store(value: any): number
 }
@@ -1556,6 +1564,9 @@ function createObjectStash(): ObjectStash {
   const map = new Map<number, any>()
   let nextID = 0
   return {
+    clear() {
+      map.clear()
+    },
     load(id) {
       return map.get(id)
     },