CRITICAL_SECTION Question
-
I have serial application, that has it's own thread that polls the serial devices. The thread reads values from the devices and storages them in a private variable. In that thread, when accessing that private variable I critical lock and unlock the variable. I also have public member function that reads that private data member. The member function is used to provide the application with access to value of the private data member. This read does not occur in the same thread. However, because i am only reading this variable is it still necessary to critical lock and unlock this variable in the public member function?
void LENZE_8200_MOTION::ThreadRun(void) { while(1) { /......./ case LNZ82_OUTPUT_CURRENT: cResults = get_output_current(lift_address, &dTemp ); // This function takes 2-3secs to complete this is way the dTemp is passed in and not m_dLiftOutputCurrent EnterCriticalSection(&m_csLiftLock); // acquires ownership of a critical section m_dLiftOutputCurrent = dTemp; if( cResults >= 0 ){ lift_cmd.Pop( &cmd );// Removes pending command from the to be process que m_lLiftFailedTxPktSeqCount = 0;// Clears Increments Connectivity Failed Packet Count }else{ m_lLiftFailedTxPktSeqCount++;// Increments Connectivity Failed Packet Count m_lLiftFailedTxPktAccumCount++;// Increments the total failed pkt count } LeaveCriticalSection(&m_csLiftLock);// releases ownership of a critical section break; case LNZ82_OPERATING_TEMPERATURE: cResults = get_temperature(lift_address, &dTemp );// This function takes 2-3secs to complete this is way the dTemp is passed in and not m_dLiftOutputTemperature EnterCriticalSection(&m_csLiftLock); // acquires ownership of a critical section m_dLiftOutputTemperature = dTemp; if( cResults >= 0 ){ lift_cmd.Pop( &cmd );// Removes pending command from the to be process que m_lLiftFailedTxPktSeqCount = 0;// Clears Increments Connectivity Failed Packet Count }else{ m_lLiftFailedTxPktSeqCount++;// Increments Connectivity Failed Packet Count m_lLiftFailedTxPktAccumCount++;// Increments the total failed pkt count m_dLiftOutputTemperature = -1; } LeaveCriticalSection(&m_csLiftLock); // releases ownership of a critical section break; /......../ } } double LENZE_8200_MOTION::lift_get_output_current(void) { double temp; ???????? Do i need critical lock here temp = m_dLiftOutputCurrent; ???????? Do i need critical unlock here re
-
I have serial application, that has it's own thread that polls the serial devices. The thread reads values from the devices and storages them in a private variable. In that thread, when accessing that private variable I critical lock and unlock the variable. I also have public member function that reads that private data member. The member function is used to provide the application with access to value of the private data member. This read does not occur in the same thread. However, because i am only reading this variable is it still necessary to critical lock and unlock this variable in the public member function?
void LENZE_8200_MOTION::ThreadRun(void) { while(1) { /......./ case LNZ82_OUTPUT_CURRENT: cResults = get_output_current(lift_address, &dTemp ); // This function takes 2-3secs to complete this is way the dTemp is passed in and not m_dLiftOutputCurrent EnterCriticalSection(&m_csLiftLock); // acquires ownership of a critical section m_dLiftOutputCurrent = dTemp; if( cResults >= 0 ){ lift_cmd.Pop( &cmd );// Removes pending command from the to be process que m_lLiftFailedTxPktSeqCount = 0;// Clears Increments Connectivity Failed Packet Count }else{ m_lLiftFailedTxPktSeqCount++;// Increments Connectivity Failed Packet Count m_lLiftFailedTxPktAccumCount++;// Increments the total failed pkt count } LeaveCriticalSection(&m_csLiftLock);// releases ownership of a critical section break; case LNZ82_OPERATING_TEMPERATURE: cResults = get_temperature(lift_address, &dTemp );// This function takes 2-3secs to complete this is way the dTemp is passed in and not m_dLiftOutputTemperature EnterCriticalSection(&m_csLiftLock); // acquires ownership of a critical section m_dLiftOutputTemperature = dTemp; if( cResults >= 0 ){ lift_cmd.Pop( &cmd );// Removes pending command from the to be process que m_lLiftFailedTxPktSeqCount = 0;// Clears Increments Connectivity Failed Packet Count }else{ m_lLiftFailedTxPktSeqCount++;// Increments Connectivity Failed Packet Count m_lLiftFailedTxPktAccumCount++;// Increments the total failed pkt count m_dLiftOutputTemperature = -1; } LeaveCriticalSection(&m_csLiftLock); // releases ownership of a critical section break; /......../ } } double LENZE_8200_MOTION::lift_get_output_current(void) { double temp; ???????? Do i need critical lock here temp = m_dLiftOutputCurrent; ???????? Do i need critical unlock here re
Yes. The read is not guaranteed to be atomic. I use them every place I read and write critical variables that might be modified or read from different threads. No problems with the software that way. Even more important with data longer than the size of a CPU register.
-
I have serial application, that has it's own thread that polls the serial devices. The thread reads values from the devices and storages them in a private variable. In that thread, when accessing that private variable I critical lock and unlock the variable. I also have public member function that reads that private data member. The member function is used to provide the application with access to value of the private data member. This read does not occur in the same thread. However, because i am only reading this variable is it still necessary to critical lock and unlock this variable in the public member function?
void LENZE_8200_MOTION::ThreadRun(void) { while(1) { /......./ case LNZ82_OUTPUT_CURRENT: cResults = get_output_current(lift_address, &dTemp ); // This function takes 2-3secs to complete this is way the dTemp is passed in and not m_dLiftOutputCurrent EnterCriticalSection(&m_csLiftLock); // acquires ownership of a critical section m_dLiftOutputCurrent = dTemp; if( cResults >= 0 ){ lift_cmd.Pop( &cmd );// Removes pending command from the to be process que m_lLiftFailedTxPktSeqCount = 0;// Clears Increments Connectivity Failed Packet Count }else{ m_lLiftFailedTxPktSeqCount++;// Increments Connectivity Failed Packet Count m_lLiftFailedTxPktAccumCount++;// Increments the total failed pkt count } LeaveCriticalSection(&m_csLiftLock);// releases ownership of a critical section break; case LNZ82_OPERATING_TEMPERATURE: cResults = get_temperature(lift_address, &dTemp );// This function takes 2-3secs to complete this is way the dTemp is passed in and not m_dLiftOutputTemperature EnterCriticalSection(&m_csLiftLock); // acquires ownership of a critical section m_dLiftOutputTemperature = dTemp; if( cResults >= 0 ){ lift_cmd.Pop( &cmd );// Removes pending command from the to be process que m_lLiftFailedTxPktSeqCount = 0;// Clears Increments Connectivity Failed Packet Count }else{ m_lLiftFailedTxPktSeqCount++;// Increments Connectivity Failed Packet Count m_lLiftFailedTxPktAccumCount++;// Increments the total failed pkt count m_dLiftOutputTemperature = -1; } LeaveCriticalSection(&m_csLiftLock); // releases ownership of a critical section break; /......../ } } double LENZE_8200_MOTION::lift_get_output_current(void) { double temp; ???????? Do i need critical lock here temp = m_dLiftOutputCurrent; ???????? Do i need critical unlock here re
The only situation where it can be justified to answer 'no' to that question would be if the variables was never written to. Since you are writing to the variables at different places in your code snippet, you have to protect the data, i.e. you need the calls to
::EnterCriticalSection()
and::LeaveCriticalSection()
at the places you were wondering about.
"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown