From 5f585987f5887ac82311835e2480960842e14402 Mon Sep 17 00:00:00 2001 From: Max Weber Date: Thu, 2 Apr 2020 00:07:17 -0600 Subject: [PATCH] Notifier: Reuse Clip instances This was leaking instances of Clip, which on linux/alsa+alsa-pulseaudio keeps a connection to pulseaudio open forever, for each notification which will eventually lock/crash the pulse daemon. If we just made it close the clip, it would become difficult to change the volume because the volume interface would go away as soon as the clip has stopped playing, so instead we keep the clip around after it has been loaded if the mtime of the file hasn't changed. --- .../java/net/runelite/client/Notifier.java | 92 +++++++++++++------ 1 file changed, 62 insertions(+), 30 deletions(-) diff --git a/runelite-client/src/main/java/net/runelite/client/Notifier.java b/runelite-client/src/main/java/net/runelite/client/Notifier.java index 5a61dd592a..cdb1d63cfe 100644 --- a/runelite-client/src/main/java/net/runelite/client/Notifier.java +++ b/runelite-client/src/main/java/net/runelite/client/Notifier.java @@ -104,6 +104,10 @@ public class Notifier private static final String appName = RuneLiteProperties.getTitle(); + private static final File NOTIFICATION_FILE = new File(RuneLite.RUNELITE_DIR, "notification.wav"); + private static final long CLIP_MTIME_UNLOADED = -2; + private static final long CLIP_MTIME_BUILTIN = -1; + private final Client client; private final RuneLiteConfig runeLiteConfig; private final ClientUI clientUI; @@ -114,6 +118,8 @@ public class Notifier private final boolean terminalNotifierAvailable; private Instant flashStart; private long mouseLastPressedMillis; + private long lastClipMTime = CLIP_MTIME_UNLOADED; + private Clip clip = null; @Inject private Notifier( @@ -408,47 +414,73 @@ public class Notifier } } - private void playCustomSound() + private synchronized void playCustomSound() { - Clip clip = null; - - // Try to load the user sound from ~/.runelite/notification.wav - File file = new File(RuneLite.RUNELITE_DIR, "notification.wav"); - if (file.exists()) + long currentMTime = NOTIFICATION_FILE.exists() ? NOTIFICATION_FILE.lastModified() : CLIP_MTIME_BUILTIN; + if (clip == null || currentMTime != lastClipMTime || !clip.isOpen()) { + if (clip != null) + { + clip.close(); + } + try - { - InputStream fileStream = new BufferedInputStream(new FileInputStream(file)); - try (AudioInputStream sound = AudioSystem.getAudioInputStream(fileStream)) - { - clip = AudioSystem.getClip(); - clip.open(sound); - } - } - catch (UnsupportedAudioFileException | IOException | LineUnavailableException e) - { - clip = null; - log.warn("Unable to play notification sound", e); - } - } - - if (clip == null) - { - // Otherwise load from the classpath - InputStream fileStream = new BufferedInputStream(Notifier.class.getResourceAsStream("notification.wav")); - try (AudioInputStream sound = AudioSystem.getAudioInputStream(fileStream)) { clip = AudioSystem.getClip(); - clip.open(sound); } - catch (UnsupportedAudioFileException | IOException | LineUnavailableException e) + catch (LineUnavailableException e) { - log.warn("Unable to play builtin notification sound", e); + lastClipMTime = CLIP_MTIME_UNLOADED; + log.warn("Unable to play notification", e); + Toolkit.getDefaultToolkit().beep(); + return; + } + lastClipMTime = currentMTime; + + if (!tryLoadNotification()) + { Toolkit.getDefaultToolkit().beep(); return; } } - clip.start(); + + // Using loop instead of start + setFramePosition prevents a the clip + // from not being played sometimes, presumably a race condition in the + // underlying line driver + clip.loop(1); + } + + private boolean tryLoadNotification() + { + if (NOTIFICATION_FILE.exists()) + { + try + { + InputStream fileStream = new BufferedInputStream(new FileInputStream(NOTIFICATION_FILE)); + try (AudioInputStream sound = AudioSystem.getAudioInputStream(fileStream)) + { + clip.open(sound); + return true; + } + } + catch (UnsupportedAudioFileException | IOException | LineUnavailableException e) + { + log.warn("Unable to load notification sound", e); + } + } + + // Otherwise load from the classpath + InputStream fileStream = new BufferedInputStream(Notifier.class.getResourceAsStream("notification.wav")); + try (AudioInputStream sound = AudioSystem.getAudioInputStream(fileStream)) + { + clip.open(sound); + return true; + } + catch (UnsupportedAudioFileException | IOException | LineUnavailableException e) + { + log.warn("Unable to load builtin notification sound", e); + } + return false; } }