Skip to content

Fix memory leak in SpotifyApi via Unmanaged ThreadLocal (Resolves #450) #451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
QiuYucheng2003 wants to merge 2 commits into
base: main
Choose a base branch
from
+25 −3
Open

Fix memory leak in SpotifyApi via Unmanaged ThreadLocal (Resolves #450) #451

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09d6053
Fix memory leak in SpotifyApi by replacing ThreadLocal<SimpleDateForm…
Feb 20, 2026
ffe93e3
Address code review suggestions for date parsing
QiuYucheng2003 Feb 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions src/main/java/se/michaelthelin/spotify/SpotifyApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
import java.util.Date;
import java.util.TimeZone;
import java.util.logging.Logger;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.time.LocalDateTime;
import java.time.format.DateTimeParseException;

/**
* Instances of the SpotifyApi class provide access to the Spotify Web API.
Expand Down Expand Up @@ -93,7 +97,9 @@ public class SpotifyApi {
* The date format used by the Spotify Web API. It uses the {@code GMT} timezone and the following pattern:
* {@code yyyy-MM-dd'T'HH:mm:ss}
*/
private static final ThreadLocal<SimpleDateFormat> SIMPLE_DATE_FORMAT = ThreadLocal.withInitial(() -> makeSimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", "GMT"));
private static final DateTimeFormatter SIMPLE_DATE_FORMATTER = DateTimeFormatter
.ofPattern("yyyy-MM-dd'T'HH:mm:ss")
Comment on lines +100 to +101
Copy link
Copy Markdown

@JackPGreen JackPGreen Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static final DateTimeFormatter SIMPLE_DATE_FORMATTER = DateTimeFormatter
.ofPattern("yyyy-MM-dd'T'HH:mm:ss")
private static final DateTimeFormatter SIMPLE_DATE_FORMATTER = DateTimeFormatter.ISO_LOCAL_DATE

See Javadoc.

Copy link
Copy Markdown
Member

@dargmuesli dargmuesli Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @QiuYucheng2003 what do you think?

Copy link
Copy Markdown
Author

@QiuYucheng2003 QiuYucheng2003 Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jack's idea to use a built-in formatter is spot on. However, ISO_LOCAL_DATE only parses the date part and will fail on our yyyy-MM-dd'T'HH:mm:ss format. We should use DateTimeFormatter.ISO_LOCAL_DATE_TIME instead to keep the time component.

I'll push a quick update to use ISO_LOCAL_DATE_TIME. I'll also include a quick null check (good catch by Copilot) to ensure it throws a ParseException instead of an NPE, maintaining 100% backward compatibility.

I will get this updated shortly.

.withZone(ZoneId.of("GMT"));

private final IHttpManager httpManager;
private final String scheme;
Expand Down Expand Up @@ -163,7 +169,23 @@ public static String concat(String[] parts, char character) {
* @throws ParseException if the date is not in a valid format
*/
public static Date parseDefaultDate(String date) throws ParseException {
return SIMPLE_DATE_FORMAT.get().parse(date);
if (date == null) {
throw new ParseException("Date string is null", 0);
}

String parsedDate = date.length() > 19 ? date.substring(0, 19) : date;

try {
return Date.from(LocalDateTime.parse(parsedDate, SIMPLE_DATE_FORMATTER)
.atZone(SIMPLE_DATE_FORMATTER.getZone())
.toInstant());
} catch (DateTimeParseException e) {
int errorIndex = e.getErrorIndex();
if (errorIndex < 0) {
errorIndex = 0;
}
throw new ParseException(e.getMessage(), errorIndex);
}
}

/**
Expand All @@ -173,7 +195,7 @@ public static Date parseDefaultDate(String date) throws ParseException {
* @return the formatted date
*/
public static String formatDefaultDate(Date date) {
return SIMPLE_DATE_FORMAT.get().format(date);
return SIMPLE_DATE_FORMATTER.format(date.toInstant());
}

/**
Expand Down