Synchronize on_ring_buffer_data_callback and writeRingbufferFiles
on_ring_buffer_data_callback callback is invoked on getting the
corresponding driver/firmware ringbuffer logs. This callback further
stores the ringbuffer data locally in cur_buffer. While the data is
getting updated through this callback, writeRingbufferFilesInternal
can get triggered from a different context which accesses the same buffer
-cur_buffer.
Synchronization is missing between these two contexts while accessing the
buffer and this commit attempts the same.
Bug: 153970986
Test: Manual - collect the bugreport and check the ring buffer logs.
Change-Id: Id99a517f4d72872b84b48c3df75dd29743f3e9b2
diff --git a/wifi/1.4/default/wifi_chip.cpp b/wifi/1.4/default/wifi_chip.cpp
index d64dfbf..23dd13b 100644
--- a/wifi/1.4/default/wifi_chip.cpp
+++ b/wifi/1.4/default/wifi_chip.cpp
@@ -1314,13 +1314,18 @@
LOG(ERROR) << "Error converting ring buffer status";
return;
}
- const auto& target = shared_ptr_this->ringbuffer_map_.find(name);
- if (target != shared_ptr_this->ringbuffer_map_.end()) {
- Ringbuffer& cur_buffer = target->second;
- cur_buffer.append(data);
- } else {
- LOG(ERROR) << "Ringname " << name << " not found";
- return;
+ {
+ std::unique_lock<std::mutex> lk(shared_ptr_this->lock_t);
+ const auto& target =
+ shared_ptr_this->ringbuffer_map_.find(name);
+ if (target != shared_ptr_this->ringbuffer_map_.end()) {
+ Ringbuffer& cur_buffer = target->second;
+ cur_buffer.append(data);
+ } else {
+ LOG(ERROR) << "Ringname " << name << " not found";
+ return;
+ }
+ // unlock
}
};
legacy_hal::wifi_error legacy_status =
@@ -1595,25 +1600,29 @@
return false;
}
// write ringbuffers to file
- for (const auto& item : ringbuffer_map_) {
- const Ringbuffer& cur_buffer = item.second;
- if (cur_buffer.getData().empty()) {
- continue;
- }
- const std::string file_path_raw =
- kTombstoneFolderPath + item.first + "XXXXXXXXXX";
- const int dump_fd = mkstemp(makeCharVec(file_path_raw).data());
- if (dump_fd == -1) {
- PLOG(ERROR) << "create file failed";
- return false;
- }
- unique_fd file_auto_closer(dump_fd);
- for (const auto& cur_block : cur_buffer.getData()) {
- if (write(dump_fd, cur_block.data(),
- sizeof(cur_block[0]) * cur_block.size()) == -1) {
- PLOG(ERROR) << "Error writing to file";
+ {
+ std::unique_lock<std::mutex> lk(lock_t);
+ for (const auto& item : ringbuffer_map_) {
+ const Ringbuffer& cur_buffer = item.second;
+ if (cur_buffer.getData().empty()) {
+ continue;
+ }
+ const std::string file_path_raw =
+ kTombstoneFolderPath + item.first + "XXXXXXXXXX";
+ const int dump_fd = mkstemp(makeCharVec(file_path_raw).data());
+ if (dump_fd == -1) {
+ PLOG(ERROR) << "create file failed";
+ return false;
+ }
+ unique_fd file_auto_closer(dump_fd);
+ for (const auto& cur_block : cur_buffer.getData()) {
+ if (write(dump_fd, cur_block.data(),
+ sizeof(cur_block[0]) * cur_block.size()) == -1) {
+ PLOG(ERROR) << "Error writing to file";
+ }
}
}
+ // unlock
}
return true;
}
diff --git a/wifi/1.4/default/wifi_chip.h b/wifi/1.4/default/wifi_chip.h
index 3323ade..98e18bb 100644
--- a/wifi/1.4/default/wifi_chip.h
+++ b/wifi/1.4/default/wifi_chip.h
@@ -19,6 +19,7 @@
#include <list>
#include <map>
+#include <mutex>
#include <android-base/macros.h>
#include <android/hardware/wifi/1.4/IWifiChip.h>
@@ -272,6 +273,7 @@
bool is_valid_;
// Members pertaining to chip configuration.
uint32_t current_mode_id_;
+ std::mutex lock_t;
std::vector<IWifiChip::ChipMode> modes_;
// The legacy ring buffer callback API has only a global callback
// registration mechanism. Use this to check if we have already