Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ActiveThread data race #4310

Closed
aleks-f opened this issue Nov 28, 2023 · 3 comments
Closed

ActiveThread data race #4310

aleks-f opened this issue Nov 28, 2023 · 3 comments
Assignees
@aleks-f
Labels
bug development fixed
Milestone
Release 1.13.0

Comments

@aleks-f
Copy link
Member

aleks-f commented Nov 28, 2023

Describe the bug
Data race due to use of naked pointers.

To Reproduce
Thread sanitizer

Expected behavior
Clean run

Logs

WARNING: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) (pid=69547)
  Write of size 8 at 0x00010b11a730 by main thread:
    #0 Poco::RefCountedObject::~RefCountedObject() RefCountedObject.cpp:27 (libPocoFoundationd.100.dylib:arm64+0x1a09f8)
    #1 Poco::Notification::~Notification() Notification.cpp:29 (libPocoFoundationd.100.dylib:arm64+0x103b80)
    #2 Poco::NewActionNotification::~NewActionNotification() ActiveThreadPool.cpp:37 (libPocoFoundationd.100.dylib:arm64+0x1b004)
    #3 Poco::NewActionNotification::~NewActionNotification() ActiveThreadPool.cpp:37 (libPocoFoundationd.100.dylib:arm64+0x1ad78)
    #4 Poco::NewActionNotification::~NewActionNotification() ActiveThreadPool.cpp:37 (libPocoFoundationd.100.dylib:arm64+0x1adc0)
    #5 Poco::RefCountedObject::release() const RefCountedObject.h:82 (testrunnerd:arm64+0x100008908)
    #6 Poco::AutoPtr<Poco::NewActionNotification>::~AutoPtr() AutoPtr.h:95 (libPocoFoundationd.100.dylib:arm64+0x1656c)
    #7 Poco::ActiveThread::start(Poco::Thread::Priority, Poco::Runnable&) ActiveThreadPool.cpp:118 (libPocoFoundationd.100.dylib:arm64+0x162f0)
    #8 Poco::ActiveThreadPool::start(Poco::Runnable&) ActiveThreadPool.cpp:255 (libPocoFoundationd.100.dylib:arm64+0x17cc0)
    #9 Poco::ActiveStarter<Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>>::start(Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>*, Poco::AutoPtr<Poco::ActiveRunnableBase>) ActiveStarter.h:39 (testrunnerd:arm64+0x1002da8a4)
    #10 CppUnit::TestRunner::run(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, std::__1::function<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> (std::exception const&)> const&) TestRunner.cpp:142 (libCppUnitd.1.dylib:arm64+0x12668)
    #11 main Driver.cpp:17 (testrunnerd:arm64+0x1000b4dac)

  Previous read of size 8 at 0x00010b11a730 by thread T50:
    #0 Poco::ActiveThread::run() ActiveThreadPool.cpp:166 (libPocoFoundationd.100.dylib:arm64+0x16c18)
    #1 Poco::(anonymous namespace)::RunnableHolder::run() Thread.cpp:56 (libPocoFoundationd.100.dylib:arm64+0x1e8618)
    #2 Poco::ThreadImpl::runnableEntry(void*) Thread_POSIX.cpp:407 (libPocoFoundationd.100.dylib:arm64+0x1e611c)

  Location is heap block of size 48 at 0x00010b11a730 allocated by main thread:
    #0 operator new(unsigned long) <null>:123127876 (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x83de0)
    #1 Poco::AutoPtr<Poco::NewActionNotification> Poco::makeAuto<Poco::NewActionNotification, Poco::Thread::Priority&, Poco::Runnable&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&>(Poco::Thread::Priority&, Poco::Runnable&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) AutoPtr.h:399 (libPocoFoundationd.100.dylib:arm64+0x163bc)
    #2 Poco::ActiveThread::start(Poco::Thread::Priority, Poco::Runnable&) ActiveThreadPool.cpp:118 (libPocoFoundationd.100.dylib:arm64+0x162b8)
    #3 Poco::ActiveThreadPool::start(Poco::Runnable&) ActiveThreadPool.cpp:255 (libPocoFoundationd.100.dylib:arm64+0x17cc0)
    #4 Poco::ActiveStarter<Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>>::start(Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>*, Poco::AutoPtr<Poco::ActiveRunnableBase>) ActiveStarter.h:39 (testrunnerd:arm64+0x1002da8a4)
    #5 CppUnit::TestRunner::run(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, std::__1::function<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> (std::exception const&)> const&) TestRunner.cpp:142 (libCppUnitd.1.dylib:arm64+0x12668)
    #6 main Driver.cpp:17 (testrunnerd:arm64+0x1000b4dac)

Please add relevant environment information:
osx, clang TSAN build

@bas524 this is a good contribution, but please - whenever possible, make use of smart pointers, there's a good reason why they exist. and run the code through sanitizers on multiple platforms/compilers
@aleks-f aleks-f added this to the Release 1.13.0 milestone Nov 28, 2023
@aleks-f aleks-f self-assigned this Nov 28, 2023
@bas524
Copy link
Contributor

bas524 commented Nov 28, 2023

Describe the bug

Data race due to use of naked pointers.

To Reproduce

Thread sanitizer

Expected behavior

Clean run

Logs


WARNING: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) (pid=69547)

  Write of size 8 at 0x00010b11a730 by main thread:

    #0 Poco::RefCountedObject::~RefCountedObject() RefCountedObject.cpp:27 (libPocoFoundationd.100.dylib:arm64+0x1a09f8)

    #1 Poco::Notification::~Notification() Notification.cpp:29 (libPocoFoundationd.100.dylib:arm64+0x103b80)

    #2 Poco::NewActionNotification::~NewActionNotification() ActiveThreadPool.cpp:37 (libPocoFoundationd.100.dylib:arm64+0x1b004)

    #3 Poco::NewActionNotification::~NewActionNotification() ActiveThreadPool.cpp:37 (libPocoFoundationd.100.dylib:arm64+0x1ad78)

    #4 Poco::NewActionNotification::~NewActionNotification() ActiveThreadPool.cpp:37 (libPocoFoundationd.100.dylib:arm64+0x1adc0)

    #5 Poco::RefCountedObject::release() const RefCountedObject.h:82 (testrunnerd:arm64+0x100008908)

    #6 Poco::AutoPtr<Poco::NewActionNotification>::~AutoPtr() AutoPtr.h:95 (libPocoFoundationd.100.dylib:arm64+0x1656c)

    #7 Poco::ActiveThread::start(Poco::Thread::Priority, Poco::Runnable&) ActiveThreadPool.cpp:118 (libPocoFoundationd.100.dylib:arm64+0x162f0)

    #8 Poco::ActiveThreadPool::start(Poco::Runnable&) ActiveThreadPool.cpp:255 (libPocoFoundationd.100.dylib:arm64+0x17cc0)

    #9 Poco::ActiveStarter<Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>>::start(Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>*, Poco::AutoPtr<Poco::ActiveRunnableBase>) ActiveStarter.h:39 (testrunnerd:arm64+0x1002da8a4)

    #10 CppUnit::TestRunner::run(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, std::__1::function<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> (std::exception const&)> const&) TestRunner.cpp:142 (libCppUnitd.1.dylib:arm64+0x12668)

    #11 main Driver.cpp:17 (testrunnerd:arm64+0x1000b4dac)



  Previous read of size 8 at 0x00010b11a730 by thread T50:

    #0 Poco::ActiveThread::run() ActiveThreadPool.cpp:166 (libPocoFoundationd.100.dylib:arm64+0x16c18)

    #1 Poco::(anonymous namespace)::RunnableHolder::run() Thread.cpp:56 (libPocoFoundationd.100.dylib:arm64+0x1e8618)

    #2 Poco::ThreadImpl::runnableEntry(void*) Thread_POSIX.cpp:407 (libPocoFoundationd.100.dylib:arm64+0x1e611c)



  Location is heap block of size 48 at 0x00010b11a730 allocated by main thread:

    #0 operator new(unsigned long) <null>:123127876 (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x83de0)

    #1 Poco::AutoPtr<Poco::NewActionNotification> Poco::makeAuto<Poco::NewActionNotification, Poco::Thread::Priority&, Poco::Runnable&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&>(Poco::Thread::Priority&, Poco::Runnable&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) AutoPtr.h:399 (libPocoFoundationd.100.dylib:arm64+0x163bc)

    #2 Poco::ActiveThread::start(Poco::Thread::Priority, Poco::Runnable&) ActiveThreadPool.cpp:118 (libPocoFoundationd.100.dylib:arm64+0x162b8)

    #3 Poco::ActiveThreadPool::start(Poco::Runnable&) ActiveThreadPool.cpp:255 (libPocoFoundationd.100.dylib:arm64+0x17cc0)

    #4 Poco::ActiveStarter<Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>>::start(Poco::AbstractEvent<int, Poco::FIFOStrategy<int, Poco::AbstractDelegate<int>>, Poco::AbstractDelegate<int>, Poco::FastMutex>*, Poco::AutoPtr<Poco::ActiveRunnableBase>) ActiveStarter.h:39 (testrunnerd:arm64+0x1002da8a4)

    #5 CppUnit::TestRunner::run(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, std::__1::function<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> (std::exception const&)> const&) TestRunner.cpp:142 (libCppUnitd.1.dylib:arm64+0x12668)

    #6 main Driver.cpp:17 (testrunnerd:arm64+0x1000b4dac)

Please add relevant environment information:

osx, clang TSAN build

@bas524 this is a good contribution, but please - whenever possible, make use of smart pointers, there's a good reason why they exist. and run the code through sanitizers on multiple platforms/compilers

Thank you. I'll try to fix it. Mistakes make our life more interesting 😁

aleks-f added a commit that referenced this issue Nov 28, 2023
@aleks-f
fix(ActiveThread): data race #4310
f061d56
@aleks-f
Copy link
Member Author

aleks-f commented Nov 28, 2023

Thank you. I'll try to fix it. Mistakes make our life more interesting 😁

it is fixed already, I was just communicating for the future.

@bas524
Copy link
Contributor

bas524 commented Nov 28, 2023

Thank you. I'll try to fix it. Mistakes make our life more interesting 😁

it is fixed already, I was just communicating for the future.

Oh, ok. I understand.

matejk added a commit that referenced this issue Dec 9, 2023
@aleks-f @matejk
4307/8/9/10 data races (#4312)
1e90f64 
* fix(NumericString): properly mark uIntToString deprecated #4304

* dev(runLibtests): allow to specify test to run

* fix(NotificationCenter): data race #4307

* fix(DirectoryWatcher): data race #4308

* fix(ArchiveStrategy): data race #4309

* fix(ActiveThread): data race #4310

* fix(Task): Cancelled Task shouldn't start running #4311 (WIP)

* fix(String): ignore clang loop unrolling warnings

* fix(TaskManager): task ownership #4311

* chore(FIFOEventTest): fix unused var warning; disable benchmark in test

* fix(Task): remove unnecessary mutex (and prevent cyclic locking reported by TSAN)

* fix(CryptoTest): disable testEncryptDecryptGCM

* fix(ci): typo

* fix(NotificationCenter): disable and clear observers in dtor (#4307)

---------

Co-authored-by: Matej Kenda <matejken@gmail.com>
@matejk matejk added the fixed label Dec 9, 2023
@aleks-f aleks-f closed this as completed Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aleks-f aleks-f

Labels
bug development fixed
Projects
None yet
Milestone
Release 1.13.0
Development

No branches or pull requests
3 participants
@matejk @bas524 @aleks-f