Skip to content

wechat_qrcode: fix Counted reference count data race - #4198

Open
zyc2 wants to merge 1 commit into
opencv:4.xfrom
zyc2:zyc2/wechat_qrcode-fix-refcount-race
Open

wechat_qrcode: fix Counted reference count data race#4198
zyc2 wants to merge 1 commit into
opencv:4.xfrom
zyc2:zyc2/wechat_qrcode-fix-refcount-race

Conversation

@zyc2

@zyc2 zyc2 commented Aug 24, 2026

Copy link
Copy Markdown

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

Description

The reference counter in zxing::Counted was not safe for concurrent access.

Under concurrent usage, the reference count could become inconsistent during
retain() / release() operations, potentially causing invalid object
lifetime management and heap corruption.

The issue was observed when using wechat_qrcode as a service. Both C++ and
Python bindings could trigger the crash under concurrent workloads, with errors
such as:
malloc(): unsorted double linked list corrupted
corrupted double-linked list

Fix

This change:

  • changes the reference counter from unsigned int to std::atomic<unsigned int>;
  • makes the decrement and zero-check in release() a single atomic operation.

Crash stack

corrupted double-linked list
/app/main.cpp:206
std::vector<short, std::allocator<short>>::~vector() at /usr/include/c++/9/bits/stl_vector.h:677
zxing::BitMatrix::~BitMatrix() at /app/dep/opencv_contrib/modules/wechat_qrcode/src/zxing/common/bitmatrix.cpp:114
zxing::Ref<zxing::LuminanceSource>::~Ref() at /app/dep/opencv_contrib/modules/wechat_qrcode/src/zxing/../zxing/common/counted.hpp:57
zxing::AdaptiveThresholdMeanBinarizer::~AdaptiveThresholdMeanBinarizer() at /app/dep/opencv_contrib/modules/wechat_qrcode/src/zxing/common/binarizer/adaptive_threshold_mean_binarizer.cpp:19
zxing::Counted::release() at /app/dep/opencv_contrib/modules/wechat_qrcode/src/zxing/common/counted.hpp:34
zxing::Ref<zxing::LuminanceSource>::~Ref() at /app/dep/opencv_contrib/modules/wechat_qrcode/src/zxing/common/counted.hpp:57
cv::wechat_qrcode::WeChatQRCode::Impl::decode[abi:cxx11](cv::Mat const&, std::vector<cv::Mat, std::allocator<cv::Mat>> const&, std::vector<cv::Mat, std::allocator<cv::Mat>>&)+0x28a)[0x7f676efa436a] at /app/dep/opencv_contrib/modules/wechat_qrcode/src/wechat_qrcode.cpp:160
/app/dep/opencv_build/lib/libopencv_wechat_qrcode.so.410(cv::wechat_qrcode::WeChatQRCode::detectAndDecode[abi:cxx11](cv::_InputArray const&, cv::_OutputArray const&)+0x18b)[0x7f676efa50bb]

Validation

The issue was observed in production. Before this change, the crash happened
intermittently (approximately once every few days under our workload).

After applying this change, the same workload has been running for about two
years without observing this heap corruption issue.

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The atomic counter and single decrement/zero test address the reported intrusive-refcount race without changing the surrounding Ref contract. In an eight-thread stress oracle that repeatedly copies a stable Ref 800,000 times, exact base a8e9acd reports multiple ThreadSanitizer races and finishes with a corrupted count; exact head 37ce345 is TSan-clean and finishes at the expected count of one.

I also configured and built the complete opencv_wechat_qrcode native module against OpenCV 4.15-dev on Apple arm64. All derived Counted types compiled and linked cleanly, so the atomic member does not introduce an in-tree copyability/build regression. Diff check is clean. There are currently no published GitHub check runs on the PR head.

Reviewed with OpenAI Codex assistance; I reproduced the exact base/head concurrency behavior and completed the native module build locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants