Skip to content

Commit f3e63d0

Browse files
committed
fix: memory leak in live preview legacy send handlers
1 parent 4ded191 commit f3e63d0

File tree

2 files changed

+104
-12
lines changed

2 files changed

+104
-12
lines changed

src/LiveDevelopment/BrowserScripts/RemoteFunctions.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ function RemoteFunctions(config = {}) {
1717

1818
// this is for bidirectional communication between phoenix and live preview
1919
const PhoenixComm = window._Brackets_LiveDev_PhoenixComm;
20-
PhoenixComm.registerLpFn("PH_Hello", function(param) {
20+
PhoenixComm && PhoenixComm.registerLpFn("PH_Hello", function(param) {
2121
// this is just a test function here to check if live preview. fn call is working correctly.
2222
console.log("Hello World", param);
2323
});

src/LiveDevelopment/MultiBrowserImpl/protocol/LiveDevProtocol.js

Lines changed: 103 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,84 @@ define(function (require, exports, module) {
8686

8787
/**
8888
* @private
89-
* A map of response IDs to deferreds, for messages that are awaiting responses.
90-
* @type {Object}
89+
* LRU cache of response IDs to deferreds, for messages that are awaiting responses.
90+
* Uses LRU cache to prevent unbounded memory growth.
91+
* @type {Phoenix.libs.LRUCache}
92+
*/
93+
const pendingLpResponses = new Phoenix.libs.LRUCache({
94+
max: MAX_PENDING_LP_CALLS_1000
95+
});
96+
97+
/**
98+
* @private
99+
* Reverse mapping: clientId -> Set of pending call IDs.
100+
* Used to clean up pending calls when a client disconnects.
101+
* @type {Map<number, Set<number>>}
102+
*/
103+
const pendingCallsByClient = new Map();
104+
105+
/**
106+
* Track a pending call for cleanup when client disconnects.
107+
* @param {number} fnCallID
108+
* @param {number} clientId
109+
* @param {$.Deferred} deferred
91110
*/
92-
var _responseDeferreds = {};
111+
function _trackPending(fnCallID, clientId, deferred) {
112+
pendingLpResponses.set(fnCallID, { deferred, clientId });
113+
114+
let set = pendingCallsByClient.get(clientId);
115+
if (!set) {
116+
set = new Set();
117+
pendingCallsByClient.set(clientId, set);
118+
}
119+
set.add(fnCallID);
120+
}
121+
122+
/**
123+
* Untrack a pending call (cleanup from both maps).
124+
* @param {number} fnCallID
125+
*/
126+
function _untrackPending(fnCallID) {
127+
const pendingHandler = pendingLpResponses.get(fnCallID);
128+
if (!pendingHandler) {
129+
return;
130+
}
131+
132+
// Delete from main cache
133+
pendingLpResponses.delete(fnCallID);
134+
135+
// Clean up reverse mapping
136+
if (pendingHandler.clientId) {
137+
const set = pendingCallsByClient.get(pendingHandler.clientId);
138+
if (set) {
139+
set.delete(fnCallID);
140+
if (set.size === 0) {
141+
pendingCallsByClient.delete(pendingHandler.clientId);
142+
}
143+
}
144+
}
145+
146+
return pendingHandler;
147+
}
148+
149+
/**
150+
* Reject all pending calls for a Live Preview client that disconnected.
151+
* @param {number} clientId
152+
*/
153+
function rejectAllPendingForClient(clientId) {
154+
const set = pendingCallsByClient.get(clientId);
155+
if (!set || set.size === 0) {
156+
return;
157+
}
158+
159+
const callIds = Array.from(set);
160+
for (const fnCallID of callIds) {
161+
const pendingHandler = _untrackPending(fnCallID);
162+
if (pendingHandler) {
163+
pendingHandler.deferred.reject(new Error(`Live Preview client disconnected: ${clientId}`));
164+
}
165+
}
166+
}
93167

94168
let _remoteFunctionProvider = null;
95169

@@ -238,7 +312,6 @@ define(function (require, exports, module) {
238312
function _receive(clientId, msgStr, messageID) {
239313
const msg = JSON.parse(msgStr),
240314
event = msg.method || "event";
241-
let deferred;
242315
if(messageID && processedMessageIDs.has(messageID)){
243316
return; // this message is already processed.
244317
} else if (messageID) {
@@ -256,13 +329,12 @@ define(function (require, exports, module) {
256329
}
257330

258331
if (msg.id) {
259-
deferred = _responseDeferreds[msg.id];
260-
if (deferred) {
261-
delete _responseDeferreds[msg.id];
332+
const pendingHandler = _untrackPending(msg.id);
333+
if (pendingHandler) {
262334
if (msg.error) {
263-
deferred.reject(msg);
335+
pendingHandler.deferred.reject(msg);
264336
} else {
265-
deferred.resolve(msg);
337+
pendingHandler.deferred.resolve(msg);
266338
}
267339
}
268340
} else if (msg.clicked && msg.tagId) {
@@ -280,7 +352,7 @@ define(function (require, exports, module) {
280352
* Dispatches a message to the remote protocol handler via the transport.
281353
*
282354
* @param {Object} msg The message to send.
283-
* @param {number|Array.<number>} idOrArray ID or IDs of the client(s) that should
355+
* @param {number|Array.<number>} clients ID or IDs of the client(s) that should
284356
* receive the message.
285357
* @return {$.Promise} A promise that's fulfilled when the response to the message is received.
286358
*/
@@ -296,7 +368,22 @@ define(function (require, exports, module) {
296368
// broadcast if there are no specific clients
297369
clients = clients || getConnectionIds();
298370
msg.id = id;
299-
_responseDeferreds[id] = result; // todo responses deffered if size larger than 100k enttries raise metric and warn in console once every 10 seconds long only
371+
372+
// Normalize clients to array
373+
if (!Array.isArray(clients)) {
374+
clients = [clients];
375+
}
376+
377+
// If no clients available, reject immediately
378+
if (clients.length === 0) {
379+
result.reject(new Error("No live preview clients connected"));
380+
return result.promise();
381+
}
382+
383+
// Track pending call for the first client (representative)
384+
const clientId = clients[0];
385+
_trackPending(id, clientId, result);
386+
300387
_transport.send(clients, JSON.stringify(msg));
301388
return result.promise();
302389
}
@@ -329,6 +416,10 @@ define(function (require, exports, module) {
329416
if(!_connections[clientId]){
330417
return;
331418
}
419+
420+
// Reject all pending calls for this client
421+
rejectAllPendingForClient(clientId);
422+
332423
delete _connections[clientId];
333424
exports.trigger("ConnectionClose", {
334425
clientId: clientId
@@ -559,6 +650,7 @@ define(function (require, exports, module) {
559650
function closeAllConnections() {
560651
getConnectionIds().forEach(function (clientId) {
561652
close(clientId);
653+
rejectAllPendingForClient(clientId);
562654
});
563655
_connections = {};
564656
}

0 commit comments

Comments
 (0)