Skip to content

Commit 617d09c

Browse files
committed
Use syncronization to address #38
1 parent 2a8b7f1 commit 617d09c

2 files changed

Lines changed: 44 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@ This change log follows the conventions of
1515
normal `0xF0`. Thanks to [@eclab](https://github.com/eclab) for
1616
identifying this in [Issue
1717
37](https://github.com/DerekCook/CoreMidi4J/issues/37).
18+
- Synchronization is used to protect against non-thread-safe behavior
19+
by
20+
[`MidiSystem.getMidiDeviceInfo()`](https://docs.oracle.com/javase/7/docs/api/javax/sound/midi/MidiSystem.html?is-external=true#getMidiDeviceInfo())
21+
when using
22+
[`CoreMidiDeviceProvider.addNotificationListener()`](https://deepsymmetry.org/coremidi4j/apidocs/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.html#addNotificationListener(uk.co.xfactorylibrarians.coremidi4j.CoreMidiNotification))
23+
on non-Mac platforms. (To be protected you must always use
24+
[`CoreMidiDeviceProvider.getMidiDeviceInfo()`](https://deepsymmetry.org/coremidi4j/apidocs/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.html#getMidiDeviceInfo())
25+
instead of the one provided by `javax.sound.midi.MidiSystem`.)
26+
Thanks to [Mailüfterl s.r.o.](https://github.com/mailuefterl-sro)
27+
for identifying this in [Issue
28+
38](https://github.com/DerekCook/CoreMidi4J/issues/38).
1829

1930

2031
## [1.4] - 2020-02-09

CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.java

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ private static Set<List<String>> snapshotCurrentEnvironment() {
417417

418418
Set<List<String>> results = new HashSet<>();
419419

420-
for (MidiDevice.Info info : MidiSystem.getMidiDeviceInfo()) {
420+
for (MidiDevice.Info info : getSystemMidiDeviceInfo()) {
421421

422422
List<String> summary = new LinkedList<>();
423423
summary.add(info.getName());
@@ -486,6 +486,13 @@ private static void watchForChanges() {
486486
* is not running macOS, then ensure that our watcher daemon thread is running in order to
487487
* be able to deliver these notifications, since we don't have CoreMIDI to initiate them.</p>
488488
*
489+
* <p>If you are using this capability on a non-macOS system, you must be sure to only call
490+
* {@link #getMidiDeviceInfo()} on this class, and never call {@link MidiSystem#getMidiDeviceInfo()}
491+
* directly, because the latter is not thread-safe, and you can run into exceptions trying to
492+
* get information about MIDI devices if your call happens to occur at the same time that our
493+
* watcher daemon thread is gathering information. The version provided by this class uses
494+
* synchronization to avoid such conflicts.</p>
495+
*
489496
* <p>Instances of {@link CoreMidiDeviceProvider} register themselves when they are constructed,
490497
* so they can keep their lists of working CoreMIDI-backed devices up to date. We only keep the
491498
* most recent one, since the Java MIDI subsystem will create many, and we only want to update
@@ -684,23 +691,40 @@ public static String getLibraryVersion() {
684691
}
685692

686693
/**
687-
* Obtains an array of information objects representing the set of all working MIDI devices available on the system.
688-
* This is a replacement for javax.sound.midi.MidiSystem.getMidiDeviceInfo(), and only returns fully-functional
689-
* MIDI devices. If you call it on a non-Mac system, it simply delegates to the javax.sound.midi implementation.
694+
* Call the JDK's raw MIDI device information collection method, while holding a lock to make sure that
695+
* this is never happening on more than one thread, which evidently can cause problems on some operating
696+
* systems.
697+
*
698+
* @return an array of <code>MidiDevice.Info</code> objects, one
699+
* for each installed MIDI device. If no such devices are installed,
700+
* an array of length 0 is returned.
701+
*/
702+
private static synchronized MidiDevice.Info[] getSystemMidiDeviceInfo() {
703+
return MidiSystem.getMidiDeviceInfo();
704+
}
705+
706+
/**
707+
* <p>Obtains an array of information objects representing the set of all working MIDI devices available on the system.
708+
* This is a replacement for {@link MidiSystem#getMidiDeviceInfo()}, and only returns fully-functional
709+
* MIDI devices. If you call it on a non-Mac system, it simply delegates to the {@code javax.sound.midi} implementation.
690710
* On a Mac, it calls that function, but filters out the broken devices, returning only the replacement versions
691711
* that CoreMidi4J provides. So by using this method rather than the standard one, you can give your users a
692-
* menu of MIDI devices which are guaranteed to properly support MIDI System Exclusive messages.
712+
* menu of MIDI devices which are guaranteed to properly support MIDI System Exclusive messages.</p>
713+
*
714+
* <p>A returned information object can then be used to obtain the corresponding device object,
715+
* by invoking {@link MidiSystem#getMidiDevice(MidiDevice.Info)}.</p>
693716
*
694-
* A returned information object can then be used to obtain the corresponding device object,
695-
* by invoking javax.sound.midi.MidiSystem.getMidiDevice().
717+
* <p>As mentioned in {@link #addNotificationListener(CoreMidiNotification)}, if you use that method, you must
718+
* only ever call this version of {@code getMidiDeviceInfo()}, never {@link MidiSystem#getMidiDeviceInfo()},
719+
* due to thread-safety issues in the latter.</p>
696720
*
697-
* @return an array of MidiDevice.Info objects, one for each installed and fully-functional MIDI device.
721+
* @return an array of {@link MidiDevice.Info} objects, one for each installed and fully-functional MIDI device.
698722
* If no such devices are installed, an array of length 0 is returned.
699723
*/
700724

701725
public static MidiDevice.Info[] getMidiDeviceInfo() {
702726

703-
MidiDevice.Info[] allInfo = MidiSystem.getMidiDeviceInfo();
727+
MidiDevice.Info[] allInfo = getSystemMidiDeviceInfo();
704728

705729
try {
706730

0 commit comments

Comments
 (0)