From cbe451a79364df408fe46d1519ef30d445a7786e Mon Sep 17 00:00:00 2001 From: Denny Sheirer Date: Sat, 4 May 2024 05:29:54 -0400 Subject: [PATCH] #1876 Unifies reentrant lock between tuners and frequency controller. (#1901) Co-authored-by: sheirerd --- .../decode/p25/P25TrafficChannelManager.java | 2 - .../source/tuner/TunerController.java | 78 +++++++++++-------- .../source/tuner/fcd/FCDTunerController.java | 13 ++-- .../tuner/frequency/FrequencyController.java | 36 ++++----- .../HeterodyneChannelSourceManager.java | 4 +- .../manager/PassThroughSourceManager.java | 4 +- .../PolyphaseChannelSourceManager.java | 4 +- .../tuner/rtl/RTL2832TunerController.java | 23 ------ .../tuner/sdrplay/RspTunerController.java | 8 +- .../source/tuner/usb/USBTunerController.java | 8 +- 10 files changed, 79 insertions(+), 101 deletions(-) diff --git a/src/main/java/io/github/dsheirer/module/decode/p25/P25TrafficChannelManager.java b/src/main/java/io/github/dsheirer/module/decode/p25/P25TrafficChannelManager.java index 122f23da9..f436f3dbb 100644 --- a/src/main/java/io/github/dsheirer/module/decode/p25/P25TrafficChannelManager.java +++ b/src/main/java/io/github/dsheirer/module/decode/p25/P25TrafficChannelManager.java @@ -1417,8 +1417,6 @@ public void stop() mAvailablePhase1TrafficChannelQueue.clear(); mAvailablePhase2TrafficChannelQueue.clear(); -//TODO: debug - mLog.info("TCM - Stopping and clearing channel grant event maps"); mTS1ChannelGrantEventMap.clear(); mTS2ChannelGrantEventMap.clear(); } diff --git a/src/main/java/io/github/dsheirer/source/tuner/TunerController.java b/src/main/java/io/github/dsheirer/source/tuner/TunerController.java index 88816da9a..e1ae1d230 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/TunerController.java +++ b/src/main/java/io/github/dsheirer/source/tuner/TunerController.java @@ -42,15 +42,18 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * Base tuner controller implementation. + */ public abstract class TunerController implements Tunable, ISourceEventProcessor, ISourceEventListener, INativeBufferProvider, Listener, ITunerErrorListener { private final static Logger mLog = LoggerFactory.getLogger(TunerController.class); //Protects access to the native buffer broadcaster for adding, removing or checking for listener count. - protected ReentrantLock mBufferListenerLock = new ReentrantLock(); + private ReentrantLock mBufferListenerLock = new ReentrantLock(); + private ReentrantLock mLock = new ReentrantLock(); protected Broadcaster mNativeBufferBroadcaster = new Broadcaster(); - protected FrequencyController mFrequencyController; private int mMiddleUnusableHalfBandwidth; private int mMeasuredFrequencyError; @@ -74,6 +77,26 @@ public TunerController(ITunerErrorListener tunerErrorListener) mFrequencyErrorCorrectionManager = new FrequencyErrorCorrectionManager(this); } + /** + * Lock for controlling configuration access to this tuner controller and any embedded tuner to ensure that + * configuration changes happen atomically. There are at least two threads that can touch this controller: + * + * A: channel processing that changes the center tuned frequency and frequency correction. + * b: user ui thread that can touch any of the frequency correction and gain controls. + * + * Generally, we'll lock on each of the primary configuration change points - getter and setter for: + * Frequency + * Frequency Correction + * Sample Rate + * Gain Controls + * + * @return lock for controlling access to tuner controller and em + */ + public ReentrantLock getLock() + { + return mLock; + } + /** * Updates the frequency controller with the new minimum and maximum values. * @param minimum frequency Hertz @@ -85,17 +108,6 @@ public void setFrequencyExtents(long minimum, long maximum) mFrequencyController.setMaximumFrequency(maximum); } - /** - * Lock for the frequency controller. This should only be used by the channel source manager to lock access to the - * frequency controller while creating a channel source, to block multi-threaded access to the frequency controller - * which might put the center tuned frequency value in an indeterminant state. - * @return frequency controller lock - */ - public ReentrantLock getFrequencyControllerLock() - { - return mFrequencyController.getFrequencyControllerLock(); - } - /** * Frequency correction manager for this tuner controller. */ @@ -292,12 +304,12 @@ public void setFrequency(long frequency) throws SourceException { try { - getFrequencyControllerLock().lock(); + getLock().lock(); mFrequencyController.setFrequency(frequency); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } } @@ -318,12 +330,12 @@ public boolean canTune(long frequency) try { - getFrequencyControllerLock().lock(); + getLock().lock(); canTune = mFrequencyController.canTune(frequency); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } return canTune; @@ -341,12 +353,12 @@ public double getFrequencyCorrection() try { - getFrequencyControllerLock().lock(); + getLock().lock(); correction = mFrequencyController.getFrequencyCorrection(); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } return correction; @@ -356,12 +368,12 @@ public void setFrequencyCorrection(double correction) throws SourceException { try { - getFrequencyControllerLock().lock(); + getLock().lock(); mFrequencyController.setFrequencyCorrection(correction); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } } @@ -375,12 +387,12 @@ public long getMinimumFrequency() try { - getFrequencyControllerLock().lock(); + getLock().lock(); minimum = mFrequencyController.getMinimumFrequency(); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } return minimum; @@ -394,12 +406,12 @@ public void setMinimumFrequency(long minimum) { try { - getFrequencyControllerLock().lock(); + getLock().lock(); mFrequencyController.setMinimumFrequency(minimum); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } } @@ -413,12 +425,12 @@ public long getMaximumFrequency() try { - getFrequencyControllerLock().lock(); + getLock().lock(); maximum = mFrequencyController.getMaximumFrequency(); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } return maximum; @@ -432,12 +444,12 @@ public void setMaximumFrequency(long maximum) { try { - getFrequencyControllerLock().lock(); + getLock().lock(); mFrequencyController.setMaximumFrequency(maximum); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } } @@ -447,12 +459,12 @@ public long getMinTunedFrequency() throws SourceException try { - getFrequencyControllerLock().lock(); + getLock().lock(); minTuned = mFrequencyController.getFrequency() - (getUsableBandwidth() / 2); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } return minTuned; @@ -464,12 +476,12 @@ public long getMaxTunedFrequency() throws SourceException try { - getFrequencyControllerLock().lock(); + getLock().lock(); maxTuned = mFrequencyController.getFrequency() + (getUsableBandwidth() / 2); } finally { - getFrequencyControllerLock().unlock(); + getLock().unlock(); } return maxTuned; diff --git a/src/main/java/io/github/dsheirer/source/tuner/fcd/FCDTunerController.java b/src/main/java/io/github/dsheirer/source/tuner/fcd/FCDTunerController.java index 4fd132585..912b6c1d9 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/fcd/FCDTunerController.java +++ b/src/main/java/io/github/dsheirer/source/tuner/fcd/FCDTunerController.java @@ -65,7 +65,6 @@ public abstract class FCDTunerController extends TunerController private FCDConfiguration mConfiguration = new FCDConfiguration(); protected ComplexMixer mComplexMixer; - private ReentrantLock mListenerLock = new ReentrantLock(); /** * Generic FCD tuner controller - contains functionality common across both @@ -126,7 +125,7 @@ else if(getTunerType() == TunerType.FUNCUBE_DONGLE_PRO_PLUS) @Override public void addBufferListener(Listener listener) { - mListenerLock.lock(); + getLock().lock(); try { @@ -139,7 +138,7 @@ public void addBufferListener(Listener listener) } finally { - mListenerLock.unlock(); + getLock().unlock(); } } @@ -150,7 +149,7 @@ public void addBufferListener(Listener listener) @Override public void removeBufferListener(Listener listener) { - mListenerLock.lock(); + getLock().lock(); try { @@ -167,7 +166,7 @@ public void removeBufferListener(Listener listener) } finally { - mListenerLock.unlock(); + getLock().unlock(); } } @@ -460,7 +459,7 @@ public void setFCDMode(Mode mode) throws UsbException, UsbClaimException */ public void setTunedFrequency(long frequency) throws SourceException { - mListenerLock.lock(); + getLock().lock(); try { @@ -473,7 +472,7 @@ public void setTunedFrequency(long frequency) throws SourceException } finally { - mListenerLock.unlock(); + getLock().unlock(); } } diff --git a/src/main/java/io/github/dsheirer/source/tuner/frequency/FrequencyController.java b/src/main/java/io/github/dsheirer/source/tuner/frequency/FrequencyController.java index ca539d140..97cb85e89 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/frequency/FrequencyController.java +++ b/src/main/java/io/github/dsheirer/source/tuner/frequency/FrequencyController.java @@ -22,11 +22,12 @@ import io.github.dsheirer.source.InvalidFrequencyException; import io.github.dsheirer.source.SourceEvent; import io.github.dsheirer.source.SourceException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.ArrayList; import java.util.List; import java.util.concurrent.locks.ReentrantLock; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class FrequencyController { @@ -42,11 +43,6 @@ public class FrequencyController private boolean mSampleRateLocked = false; private List mProcessors = new ArrayList<>(); - /** - * Lock protecting access to frequency control plane, source event processor subscriptions and event broadcasting - */ - private ReentrantLock mLock = new ReentrantLock(); - /** * Constructs an instance * @param tunable that can be controlled by this frequency controller. @@ -56,15 +52,6 @@ public FrequencyController(Tunable tunable) mTunable = tunable; } - - /** - * Lock for controlling access to frequency control plane, event processor subscriptions and source event broadcasts. - */ - public ReentrantLock getFrequencyControllerLock() - { - return mLock; - } - /** * Prepare for disposal of this instance. */ @@ -294,7 +281,7 @@ public void setFrequencyCorrection(double correction) throws SourceException */ public void addSourceEventProcessor(ISourceEventProcessor processor) { - mLock.lock(); + mTunable.getLock().lock(); try { @@ -305,7 +292,7 @@ public void addSourceEventProcessor(ISourceEventProcessor processor) } finally { - mLock.unlock(); + mTunable.getLock().unlock(); } } @@ -314,7 +301,7 @@ public void addSourceEventProcessor(ISourceEventProcessor processor) */ public void removeSourceEventProcessor(ISourceEventProcessor processor) { - mLock.lock(); + mTunable.getLock().lock(); try { @@ -322,7 +309,7 @@ public void removeSourceEventProcessor(ISourceEventProcessor processor) } finally { - mLock.unlock(); + mTunable.getLock().unlock(); } } @@ -354,7 +341,7 @@ protected void broadcastSampleRateChange() throws SourceException public void broadcast(SourceEvent event) throws SourceException { - mLock.lock(); + mTunable.getLock().lock(); try { @@ -365,13 +352,18 @@ public void broadcast(SourceEvent event) throws SourceException } finally { - mLock.unlock(); + mTunable.getLock().unlock(); } } public interface Tunable { + /** + * Reentrant lock to synchronize threaded access to tuner controls. + */ + ReentrantLock getLock(); + /** * Gets the tuned frequency of the device */ diff --git a/src/main/java/io/github/dsheirer/source/tuner/manager/HeterodyneChannelSourceManager.java b/src/main/java/io/github/dsheirer/source/tuner/manager/HeterodyneChannelSourceManager.java index 7270d659a..793a416d1 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/manager/HeterodyneChannelSourceManager.java +++ b/src/main/java/io/github/dsheirer/source/tuner/manager/HeterodyneChannelSourceManager.java @@ -116,7 +116,7 @@ public TunerChannelSource getSource(TunerChannel tunerChannel, ChannelSpecificat try { - mTunerController.getFrequencyControllerLock().lock(); + mTunerController.getLock().lock(); if(CenterFrequencyCalculator.canTune(tunerChannel, mTunerController, mTunerChannels)) { try @@ -150,7 +150,7 @@ public TunerChannelSource getSource(TunerChannel tunerChannel, ChannelSpecificat } finally { - mTunerController.getFrequencyControllerLock().unlock(); + mTunerController.getLock().unlock(); } return source; diff --git a/src/main/java/io/github/dsheirer/source/tuner/manager/PassThroughSourceManager.java b/src/main/java/io/github/dsheirer/source/tuner/manager/PassThroughSourceManager.java index 7d9807dd1..9399973b3 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/manager/PassThroughSourceManager.java +++ b/src/main/java/io/github/dsheirer/source/tuner/manager/PassThroughSourceManager.java @@ -106,7 +106,7 @@ public TunerChannelSource getSource(TunerChannel tunerChannel, ChannelSpecificat try { - mTunerController.getFrequencyControllerLock().lock(); + mTunerController.getLock().lock(); PassThroughChannelSource channelSource = new PassThroughChannelSource(new SourceEventProxy(), mTunerController, tunerChannel, threadName); @@ -116,7 +116,7 @@ public TunerChannelSource getSource(TunerChannel tunerChannel, ChannelSpecificat } finally { - mTunerController.getFrequencyControllerLock().unlock(); + mTunerController.getLock().unlock(); } return source; diff --git a/src/main/java/io/github/dsheirer/source/tuner/manager/PolyphaseChannelSourceManager.java b/src/main/java/io/github/dsheirer/source/tuner/manager/PolyphaseChannelSourceManager.java index 2ce4c3088..57c9e7912 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/manager/PolyphaseChannelSourceManager.java +++ b/src/main/java/io/github/dsheirer/source/tuner/manager/PolyphaseChannelSourceManager.java @@ -431,7 +431,7 @@ public TunerChannelSource getSource(TunerChannel tunerChannel, ChannelSpecificat try { - mTunerController.getFrequencyControllerLock().lock(); + mTunerController.getLock().lock(); if(isTunable(tunerChannel)) { //Get a new set of currently tuned channels @@ -478,7 +478,7 @@ public TunerChannelSource getSource(TunerChannel tunerChannel, ChannelSpecificat } finally { - mTunerController.getFrequencyControllerLock().unlock(); + mTunerController.getLock().unlock(); } return tunerChannelSource; diff --git a/src/main/java/io/github/dsheirer/source/tuner/rtl/RTL2832TunerController.java b/src/main/java/io/github/dsheirer/source/tuner/rtl/RTL2832TunerController.java index 2104a62a9..de09ff7b7 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/rtl/RTL2832TunerController.java +++ b/src/main/java/io/github/dsheirer/source/tuner/rtl/RTL2832TunerController.java @@ -67,7 +67,6 @@ public class RTL2832TunerController extends USBTunerController private EmbeddedTuner mEmbeddedTuner; private long mTunedFrequency = 0; private boolean mBiasTEnabled = false; - private ReentrantLock mLock = new ReentrantLock(); /** * Abstract tuner controller device. Use the static gettype() method @@ -79,28 +78,6 @@ public RTL2832TunerController(int bus, String portAddress, ITunerErrorListener t super(bus, portAddress, tunerErrorListener); } - /** - * Lock for controlling configuration access to this tuner controller and the embedded tuner to ensure that - * configuration changes involving multiple USB control messages can happen atomically. There are at least two - * threads that can touch this controller: - * - * A: channel processing that changes the center tuned frequency and frequency correction. - * b: user ui thread that can touch any of the frequency correction and gain controls. - * - * Generally, we'll lock on each of the primary configuration change points - getter and setter for: - * Frequency - * Frequency Correction - * Sample Rate - * Gain Controls - * - * @return lock for controlling access to tuner controller and em - */ - private ReentrantLock getLock() - { - return mLock; - } - - @Override protected INativeBufferFactory getNativeBufferFactory() { diff --git a/src/main/java/io/github/dsheirer/source/tuner/sdrplay/RspTunerController.java b/src/main/java/io/github/dsheirer/source/tuner/sdrplay/RspTunerController.java index db11c8fff..2599d25ca 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/sdrplay/RspTunerController.java +++ b/src/main/java/io/github/dsheirer/source/tuner/sdrplay/RspTunerController.java @@ -185,7 +185,7 @@ public void stop() @Override public void addBufferListener(Listener listener) { - mBufferListenerLock.lock(); + getLock().lock(); try { @@ -198,7 +198,7 @@ public void addBufferListener(Listener listener) } finally { - mBufferListenerLock.unlock(); + getLock().unlock(); } } @@ -209,7 +209,7 @@ public void addBufferListener(Listener listener) @Override public void removeBufferListener(Listener listener) { - mBufferListenerLock.lock(); + getLock().lock(); try { @@ -222,7 +222,7 @@ public void removeBufferListener(Listener listener) } finally { - mBufferListenerLock.unlock(); + getLock().unlock(); } } diff --git a/src/main/java/io/github/dsheirer/source/tuner/usb/USBTunerController.java b/src/main/java/io/github/dsheirer/source/tuner/usb/USBTunerController.java index e10d34838..2be35281a 100644 --- a/src/main/java/io/github/dsheirer/source/tuner/usb/USBTunerController.java +++ b/src/main/java/io/github/dsheirer/source/tuner/usb/USBTunerController.java @@ -477,7 +477,7 @@ public void addBufferListener(Listener listener) { if(isRunning()) { - mBufferListenerLock.lock(); + getLock().lock(); try { @@ -492,7 +492,7 @@ public void addBufferListener(Listener listener) } finally { - mBufferListenerLock.unlock(); + getLock().unlock(); } } } @@ -503,7 +503,7 @@ public void addBufferListener(Listener listener) @Override public void removeBufferListener(Listener listener) { - mBufferListenerLock.lock(); + getLock().lock(); try { @@ -516,7 +516,7 @@ public void removeBufferListener(Listener listener) } finally { - mBufferListenerLock.unlock(); + getLock().unlock(); } }