Skip to content

Commit acc8804

Browse files
committed
Fix memory leaks by cleaning up object URLs in webamp package
This commit addresses object URL memory leaks by ensuring proper cleanup: 1. Track URLs: Added cleanup in tracks reducer when tracks are removed - REMOVE_TRACKS action now revokes object URLs for removed tracks - REMOVE_ALL_TRACKS action cleans up all object URLs 2. Album art URLs: Clean up old album art URLs when they are replaced - SET_MEDIA_TAGS action revokes old album art URL if being replaced 3. Skin URLs: setSkinFromUrl now detects and cleans up object URLs - Automatically revokes blob: URLs after fetch completes 4. Skin parser: fallbackGetImgFromBlob now cleans up after image loads - Object URL is revoked once image loading completes or fails These changes prevent memory leaks when users: - Add/remove tracks with blob URLs - Change tracks with album art - Load custom skins from files - Parse skin images Fixes object URL cleanup as noted in TODO comments in playlist.ts
1 parent b00e359 commit acc8804

File tree

4 files changed

+62
-5
lines changed

4 files changed

+62
-5
lines changed

packages/webamp/js/actionCreators/files.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ export function setSkinFromBlob(blob: Blob | Promise<Blob>): Thunk {
116116
export function setSkinFromUrl(url: string): Thunk {
117117
return async (dispatch) => {
118118
dispatch({ type: "LOADING" });
119+
const isObjectUrl = url.startsWith("blob:");
119120
try {
120121
const response = await fetch(url);
121122
if (!response.ok) {
@@ -126,6 +127,11 @@ export function setSkinFromUrl(url: string): Thunk {
126127
console.error(e);
127128
dispatch({ type: "LOADED" });
128129
alert(`Failed to download skin from ${url}`);
130+
} finally {
131+
// Clean up object URL if one was created
132+
if (isObjectUrl) {
133+
URL.revokeObjectURL(url);
134+
}
129135
}
130136
};
131137
}

packages/webamp/js/reducers/playlist.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ const playlist = (
7474
),
7575
};
7676
case "REMOVE_ALL_TRACKS":
77-
// TODO: Consider disposing of ObjectUrls
7877
return {
7978
...state,
8079
trackOrder: [],
@@ -83,7 +82,6 @@ const playlist = (
8382
lastSelectedIndex: null,
8483
};
8584
case "REMOVE_TRACKS":
86-
// TODO: Consider disposing of ObjectUrls
8785
const actionIds = new Set((action as any).ids.map(Number));
8886
const { currentTrack } = state;
8987
return {

packages/webamp/js/reducers/tracks.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ const tracks = (
8888
},
8989
};
9090
}
91-
case "SET_MEDIA_TAGS":
91+
case "SET_MEDIA_TAGS": {
9292
const track = state[(action as any).id];
9393
const {
9494
sampleRate,
@@ -100,6 +100,16 @@ const tracks = (
100100
albumArtUrl,
101101
} = action as any;
102102
const { kbps, khz, channels } = track;
103+
104+
// Clean up old album art URL if it's being replaced
105+
if (
106+
track.albumArtUrl &&
107+
track.albumArtUrl.startsWith("blob:") &&
108+
track.albumArtUrl !== albumArtUrl
109+
) {
110+
URL.revokeObjectURL(track.albumArtUrl);
111+
}
112+
103113
return {
104114
...state,
105115
[(action as any).id]: {
@@ -114,6 +124,42 @@ const tracks = (
114124
channels: numberOfChannels != null ? numberOfChannels : channels,
115125
},
116126
};
127+
}
128+
case "REMOVE_TRACKS": {
129+
const actionIds = (action as any).ids;
130+
const newState = { ...state };
131+
132+
// Clean up object URLs for removed tracks
133+
actionIds.forEach((id: number) => {
134+
const track = state[id];
135+
if (track) {
136+
// Revoke track URL if it's an object URL
137+
if (track.url && track.url.startsWith("blob:")) {
138+
URL.revokeObjectURL(track.url);
139+
}
140+
// Revoke album art URL if it's an object URL
141+
if (track.albumArtUrl && track.albumArtUrl.startsWith("blob:")) {
142+
URL.revokeObjectURL(track.albumArtUrl);
143+
}
144+
delete newState[id];
145+
}
146+
});
147+
148+
return newState;
149+
}
150+
case "REMOVE_ALL_TRACKS": {
151+
// Clean up all object URLs
152+
Object.values(state).forEach((track) => {
153+
if (track.url && track.url.startsWith("blob:")) {
154+
URL.revokeObjectURL(track.url);
155+
}
156+
if (track.albumArtUrl && track.albumArtUrl.startsWith("blob:")) {
157+
URL.revokeObjectURL(track.albumArtUrl);
158+
}
159+
});
160+
161+
return {};
162+
}
117163
default:
118164
return state;
119165
}

packages/webamp/js/skinParserUtils.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,15 @@ export async function getFileFromZip(
5555
}
5656
}
5757

58-
function fallbackGetImgFromBlob(blob: Blob): Promise<HTMLImageElement> {
59-
return Utils.imgFromUrl(URL.createObjectURL(blob));
58+
async function fallbackGetImgFromBlob(blob: Blob): Promise<HTMLImageElement> {
59+
const url = URL.createObjectURL(blob);
60+
try {
61+
const img = await Utils.imgFromUrl(url);
62+
return img;
63+
} finally {
64+
// Clean up the object URL after the image has loaded (or failed to load)
65+
URL.revokeObjectURL(url);
66+
}
6067
}
6168

6269
export async function getImgFromBlob(

0 commit comments

Comments
 (0)