Skip to content

Commit d6b8ce9

Browse files
committed
fix: Ref-count batch operations
+ don's close the secure channel preemptively
1 parent b8fcf84 commit d6b8ce9

File tree

4 files changed

+58
-38
lines changed

4 files changed

+58
-38
lines changed

include/keycard-qt/communication_manager.h

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -102,29 +102,22 @@ class CommunicationManager : public ICommunicationManager {
102102
void stop();
103103

104104
/**
105-
* @brief Start batch operations mode
106-
*
105+
* @brief Start batch operations mode (ref-counted)
106+
*
107107
* Prevents the communication manager from automatically stopping card detection
108108
* when the command queue becomes empty. Useful for performing multiple sequential
109109
* operations without channel stop/start cycles.
110-
*
111-
* Call endBatchOperations() when done to allow automatic detection management.
112-
*
113-
* Example:
114-
* @code
115-
* m_commMgr->startBatchOperations();
116-
* // Perform multiple executeCommandSync() calls
117-
* // Channel stays open between commands
118-
* m_commMgr->endBatchOperations();
119-
* @endcode
110+
*
111+
* Calls are ref-counted: every startBatchOperations() must be matched with a
112+
* corresponding endBatchOperations() call.
120113
*/
121114
void startBatchOperations() override;
122115

123116
/**
124-
* @brief End batch operations mode
125-
*
126-
* Re-enables automatic card detection management. If the queue is empty,
127-
* detection will be stopped.
117+
* @brief End batch operations mode (ref-counted)
118+
*
119+
* Decrements the active batch reference count. Automatic detection management
120+
* is re-enabled only when the count reaches zero.
128121
*/
129122
void endBatchOperations() override;
130123

@@ -311,8 +304,8 @@ private slots:
311304
// Running flag
312305
bool m_running;
313306

314-
// Batch operations flag - when true, don't stop detection on empty queue
315-
bool m_batchOperations;
307+
// Batch operations ref-count - > 0 means don't stop detection on empty queue
308+
int m_batchOperationDepth;
316309
QMutex m_batchMutex;
317310
};
318311

src/command_set.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,10 +1431,6 @@ void CommandSet::stopDetection() {
14311431
return;
14321432
}
14331433

1434-
// Reset secure channel when stopping detection
1435-
// This ensures that when detection restarts, the secure channel will be properly re-established
1436-
resetSecureChannel();
1437-
14381434
// This runs on main thread, so it's safe to call directly
14391435
m_channel->setState(ChannelState::Idle);
14401436
emit channelStateChanged(ChannelState::Idle);

src/communication_manager.cpp

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ CommunicationManager::CommunicationManager(QObject* parent)
3535
, m_commThread(nullptr)
3636
, m_state(State::Idle)
3737
, m_running(false)
38-
, m_batchOperations(false)
38+
, m_batchOperationDepth(0)
3939
{
4040
qDebug() << "CommunicationManager: Created";
4141
}
@@ -133,7 +133,7 @@ void CommunicationManager::stop() {
133133
// Step 2: Clear batch mode
134134
{
135135
QMutexLocker locker(&m_batchMutex);
136-
m_batchOperations = false;
136+
m_batchOperationDepth = 0;
137137
}
138138

139139
// Step 3: Stop card detection
@@ -216,26 +216,46 @@ void CommunicationManager::stop() {
216216
}
217217

218218
void CommunicationManager::startBatchOperations() {
219-
QMutexLocker locker(&m_batchMutex);
220-
if (!m_batchOperations) {
221-
m_batchOperations = true;
219+
int depth = 0;
220+
bool becameActive = false;
221+
{
222+
QMutexLocker locker(&m_batchMutex);
223+
if (m_batchOperationDepth == 0) {
224+
becameActive = true;
225+
}
226+
++m_batchOperationDepth;
227+
depth = m_batchOperationDepth;
228+
}
229+
230+
if (becameActive) {
222231
qDebug() << "CommunicationManager: Batch operations mode ENABLED - channel will stay open";
232+
} else {
233+
qDebug() << "CommunicationManager: Batch operations depth increased to" << depth;
223234
}
224235
}
225236

226237
void CommunicationManager::endBatchOperations() {
227-
bool wasBatch = false;
238+
int depth = 0;
239+
bool becameInactive = false;
228240
{
229241
QMutexLocker locker(&m_batchMutex);
230-
wasBatch = m_batchOperations;
231-
m_batchOperations = false;
242+
if (m_batchOperationDepth <= 0) {
243+
qWarning() << "CommunicationManager: endBatchOperations() called with depth" << m_batchOperationDepth;
244+
return;
245+
}
246+
247+
--m_batchOperationDepth;
248+
depth = m_batchOperationDepth;
249+
becameInactive = (m_batchOperationDepth == 0);
232250
}
233-
234-
if (wasBatch) {
251+
252+
if (becameInactive) {
235253
qDebug() << "CommunicationManager: Batch operations mode DISABLED";
236-
254+
237255
// Check if queue is empty and stop detection if needed
238256
QMetaObject::invokeMethod(this, &CommunicationManager::processQueue, Qt::QueuedConnection);
257+
} else {
258+
qDebug() << "CommunicationManager: Batch operations depth decreased to" << depth;
239259
}
240260
}
241261

@@ -600,7 +620,7 @@ void CommunicationManager::processQueue() {
600620
bool inBatchMode = false;
601621
{
602622
QMutexLocker batchLocker(&m_batchMutex);
603-
inBatchMode = m_batchOperations;
623+
inBatchMode = (m_batchOperationDepth > 0);
604624
}
605625

606626
if (inBatchMode) {

tests/test_communication_manager.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,12 +255,23 @@ private slots:
255255

256256
void testNestedBatchOperations() {
257257
m_commMgr->init(m_cmdSet);
258-
258+
259259
m_commMgr->startBatchOperations();
260-
m_commMgr->startBatchOperations(); // Second call should be idempotent
261-
260+
m_commMgr->startBatchOperations(); // Second call should increase depth
261+
262262
m_commMgr->endBatchOperations();
263-
263+
m_commMgr->endBatchOperations(); // Fully exit batch mode
264+
265+
QVERIFY(true);
266+
}
267+
268+
void testEndBatchOperationsWithoutStart() {
269+
m_commMgr->init(m_cmdSet);
270+
271+
// Extra end calls should be safe and never underflow.
272+
m_commMgr->endBatchOperations();
273+
m_commMgr->endBatchOperations();
274+
264275
QVERIFY(true);
265276
}
266277

0 commit comments

Comments
 (0)