Re: [Wireshark-dev] ThreadSanitizer issue in RecentFileStatus()
On 2/3/18 3:23 AM, Stig Bjørlykke wrote: > On Fri, Feb 2, 2018 at 7:33 PM, Gerald Combs wrote: >> That's correct -- the main and RecentFileStatus threads could operate on the >> filename at the same time. I think the data race is harmless in this case, >> but it's easy enough to create a local copy of the filename. Fix inbound at >> https://code.wireshark.org/review/#/c/25572. > > I'm still getting a similar race condition report from > RecentFileStatus. Even if it's harmless it would have been good to > fix this to make it possible to run with TSAN and "Pause on issues". > > == > WARNING: ThreadSanitizer: data race (pid=4733) > Read of size 8 at 0x7e800059edb0 by main thread: > * #0 QString::QString(QString const&) qstring.h:906 > (Wireshark:x86_64+0x100039256) > #1 QString::QString(QString const&) qstring.h:907 > (Wireshark:x86_64+0x100038498) > #2 WiresharkApplication::qt_static_metacall(QObject*, > QMetaObject::Call, int, void**) moc_wireshark_application.cpp:239 > (Wireshark:x86_64+0x1000bb990) > #3 QObject::event(QEvent*) (QtCore:x86_64+0x211b80) > #4 QApplicationPrivate::notify_helper(QObject*, QEvent*) > (QtWidgets:x86_64+0x119ac) > #5 start (libdyld.dylib:x86_64+0x1114) > > Previous write of size 8 at 0x7e800059edb0 by thread T14: > * #0 QString::QString(QString const&) qstring.h:906 > (Wireshark:x86_64+0x10003926d) > #1 QString::QString(QString const&) qstring.h:907 > (Wireshark:x86_64+0x100038498) > #2 RecentFileStatus::run() recent_file_status.cpp:32 > (Wireshark:x86_64+0x1005000cc) > #3 non-virtual thunk to RecentFileStatus::run() > recent_file_status.cpp (Wireshark:x86_64+0x10050020c) > #4 (QtCore:x86_64+0x27b6d) Does ThreadSanitizer support QMutex on macOS (which appears to use Mach semaphores)? Switching from Qt::BlockingQueuedConnection to Qt::QueuedConnection fixed the warning here, which suggests that it's ignoring Qt's event mutex. I get similar warnings for GMutex when using TSAN on Linux. A switch from Qt::BlockingQueuedConnection to Qt::QueuedConnection is inbound in change 25576. ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] ThreadSanitizer issue in RecentFileStatus()
On Fri, Feb 2, 2018 at 7:33 PM, Gerald Combs wrote: > That's correct -- the main and RecentFileStatus threads could operate on the > filename at the same time. I think the data race is harmless in this case, > but it's easy enough to create a local copy of the filename. Fix inbound at > https://code.wireshark.org/review/#/c/25572. I'm still getting a similar race condition report from RecentFileStatus. Even if it's harmless it would have been good to fix this to make it possible to run with TSAN and "Pause on issues". == WARNING: ThreadSanitizer: data race (pid=4733) Read of size 8 at 0x7e800059edb0 by main thread: * #0 QString::QString(QString const&) qstring.h:906 (Wireshark:x86_64+0x100039256) #1 QString::QString(QString const&) qstring.h:907 (Wireshark:x86_64+0x100038498) #2 WiresharkApplication::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) moc_wireshark_application.cpp:239 (Wireshark:x86_64+0x1000bb990) #3 QObject::event(QEvent*) (QtCore:x86_64+0x211b80) #4 QApplicationPrivate::notify_helper(QObject*, QEvent*) (QtWidgets:x86_64+0x119ac) #5 start (libdyld.dylib:x86_64+0x1114) Previous write of size 8 at 0x7e800059edb0 by thread T14: * #0 QString::QString(QString const&) qstring.h:906 (Wireshark:x86_64+0x10003926d) #1 QString::QString(QString const&) qstring.h:907 (Wireshark:x86_64+0x100038498) #2 RecentFileStatus::run() recent_file_status.cpp:32 (Wireshark:x86_64+0x1005000cc) #3 non-virtual thunk to RecentFileStatus::run() recent_file_status.cpp (Wireshark:x86_64+0x10050020c) #4 (QtCore:x86_64+0x27b6d) Issue is caused by frames marked with "*". Location is stack of thread T14. Thread T14 (tid=1194099, running) created by main thread at: #0 pthread_create (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x2a34d) #1 QThread::start(QThread::Priority) (QtCore:x86_64+0x2b5cb) #2 main wireshark-qt.cpp:667 (Wireshark:x86_64+0x1000366cd) SUMMARY: ThreadSanitizer: data race qstring.h:906 in QString::QString(QString const&) == -- Stig Bjørlykke ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] ThreadSanitizer issue in RecentFileStatus()
Would it make sense to either expand README.developer or add a README.threading, detailing the new startup procedure? Otherwise we might run into issues later where users might face those exact issues. On Fri, Feb 2, 2018 at 7:33 PM, Gerald Combs wrote: > That's correct -- the main and RecentFileStatus threads could operate on > the filename at the same time. I think the data race is harmless in this > case, but it's easy enough to create a local copy of the filename. Fix > inbound at https://code.wireshark.org/review/#/c/25572. > > On 2/2/18 8:08 AM, Jaap Keuter wrote: > > Hi, > > > > Good question. > > Just spitballing here, in what thread are the constructors running? > > RecentFileStatus::filename_ is being set during construction, but is > run() already called from the other thread? > > > > Jaap > > > > > >> On 2 Feb 2018, at 15:04, Stig Bjørlykke wrote: > >> > >> When running with ThreadSanitizer I constantly get this race condition > >> report from a QString in RecentFileStatus. Anyone else seen this? I > >> can't figure out the issue... > >> > >> == > >> WARNING: ThreadSanitizer: data race (pid=41949) > >> Read of size 8 at 0x7b0c000c0dd0 by thread T18: > >> * #0 QString::QString(QString const&) qstring.h:906 > >> (Wireshark:x86_64+0x100039756) > >>#1 QString::QString(QString const&) qstring.h:907 > >> (Wireshark:x86_64+0x100038998) > >>#2 RecentFileStatus::run() recent_file_status.cpp:33 > >> (Wireshark:x86_64+0x1004ffbec) > >>#3 non-virtual thunk to RecentFileStatus::run() > >> recent_file_status.cpp (Wireshark:x86_64+0x1004ffd2c) > >>#4 :8575680 (QtCore:x86_64+0x27b6d) > >> > >> Previous write of size 8 at 0x7b0c000c0dd0 by main thread: > >> * #0 QString::QString(QString const&) qstring.h:906 > >> (Wireshark:x86_64+0x10003976d) > >>#1 QString::QString(QString const&) qstring.h:907 > >> (Wireshark:x86_64+0x100038998) > >>#2 RecentFileStatus::RecentFileStatus(QString, QObject*) > >> recent_file_status.cpp:13 (Wireshark:x86_64+0x1004ff8b3) > >>#3 RecentFileStatus::RecentFileStatus(QString, QObject*) > >> recent_file_status.cpp:14 (Wireshark:x86_64+0x1004ffac0) > >>#4 WiresharkApplication::refreshRecentCaptures() > >> wireshark_application.cpp:256 (Wireshark:x86_64+0x10079a3b6) > >>#5 WiresharkApplication::qt_static_metacall(QObject*, > >> QMetaObject::Call, int, void**) > >> moc_wireshark_application_Debug.cpp:234 (Wireshark:x86_64+0x1000bbceb) > >>#6 QMetaObject::activate(QObject*, int, int, void**) > >> :8575680 (QtCore:x86_64+0x2194ba) > >>#7 main wireshark-qt.cpp:852 (Wireshark:x86_64+0x100037b0c) > >> > >> Issue is caused by frames marked with "*". > >> > >> Location is heap block of size 48 at 0x7b0c000c0db0 allocated by main > thread: > >>#0 operator new(unsigned long) :8575712 > >> (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x6ba8e) > >>#1 WiresharkApplication::refreshRecentCaptures() > >> wireshark_application.cpp:256 (Wireshark:x86_64+0x10079a36a) > >>#2 WiresharkApplication::qt_static_metacall(QObject*, > >> QMetaObject::Call, int, void**) > >> moc_wireshark_application_Debug.cpp:234 (Wireshark:x86_64+0x1000bbceb) > >>#3 QMetaObject::activate(QObject*, int, int, void**) > >> :8575712 (QtCore:x86_64+0x2194ba) > >>#4 main wireshark-qt.cpp:852 (Wireshark:x86_64+0x100037b0c) > >> > >> Thread T18 (tid=17501933, running) created by main thread at: > >>#0 pthread_create :8575760 > >> (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x2a34d) > >>#1 QThread::start(QThread::Priority) :8575760 > (QtCore:x86_64+0x2b5cb) > >>#2 main wireshark-qt.cpp:667 (Wireshark:x86_64+0x100036bcd) > >> > >> SUMMARY: ThreadSanitizer: data race qstring.h:906 in > >> QString::QString(QString const&) > >> == > >> > >> > >> -- > >> Stig Bjørlykke > > > > > ___ > > Sent via:Wireshark-dev mailing list > > Archives:https://www.wireshark.org/lists/wireshark-dev > > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > > mailto:wireshark-dev-requ...@wireshark.org?subject= > unsubscribe > > > > > ___ > Sent via:Wireshark-dev mailing list > Archives:https://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > mailto:wireshark-dev-requ...@wireshark.org?subject= > unsubscribe > ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] ThreadSanitizer issue in RecentFileStatus()
That's correct -- the main and RecentFileStatus threads could operate on the filename at the same time. I think the data race is harmless in this case, but it's easy enough to create a local copy of the filename. Fix inbound at https://code.wireshark.org/review/#/c/25572. On 2/2/18 8:08 AM, Jaap Keuter wrote: > Hi, > > Good question. > Just spitballing here, in what thread are the constructors running? > RecentFileStatus::filename_ is being set during construction, but is run() > already called from the other thread? > > Jaap > > >> On 2 Feb 2018, at 15:04, Stig Bjørlykke wrote: >> >> When running with ThreadSanitizer I constantly get this race condition >> report from a QString in RecentFileStatus. Anyone else seen this? I >> can't figure out the issue... >> >> == >> WARNING: ThreadSanitizer: data race (pid=41949) >> Read of size 8 at 0x7b0c000c0dd0 by thread T18: >> * #0 QString::QString(QString const&) qstring.h:906 >> (Wireshark:x86_64+0x100039756) >>#1 QString::QString(QString const&) qstring.h:907 >> (Wireshark:x86_64+0x100038998) >>#2 RecentFileStatus::run() recent_file_status.cpp:33 >> (Wireshark:x86_64+0x1004ffbec) >>#3 non-virtual thunk to RecentFileStatus::run() >> recent_file_status.cpp (Wireshark:x86_64+0x1004ffd2c) >>#4 :8575680 (QtCore:x86_64+0x27b6d) >> >> Previous write of size 8 at 0x7b0c000c0dd0 by main thread: >> * #0 QString::QString(QString const&) qstring.h:906 >> (Wireshark:x86_64+0x10003976d) >>#1 QString::QString(QString const&) qstring.h:907 >> (Wireshark:x86_64+0x100038998) >>#2 RecentFileStatus::RecentFileStatus(QString, QObject*) >> recent_file_status.cpp:13 (Wireshark:x86_64+0x1004ff8b3) >>#3 RecentFileStatus::RecentFileStatus(QString, QObject*) >> recent_file_status.cpp:14 (Wireshark:x86_64+0x1004ffac0) >>#4 WiresharkApplication::refreshRecentCaptures() >> wireshark_application.cpp:256 (Wireshark:x86_64+0x10079a3b6) >>#5 WiresharkApplication::qt_static_metacall(QObject*, >> QMetaObject::Call, int, void**) >> moc_wireshark_application_Debug.cpp:234 (Wireshark:x86_64+0x1000bbceb) >>#6 QMetaObject::activate(QObject*, int, int, void**) >> :8575680 (QtCore:x86_64+0x2194ba) >>#7 main wireshark-qt.cpp:852 (Wireshark:x86_64+0x100037b0c) >> >> Issue is caused by frames marked with "*". >> >> Location is heap block of size 48 at 0x7b0c000c0db0 allocated by main >> thread: >>#0 operator new(unsigned long) :8575712 >> (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x6ba8e) >>#1 WiresharkApplication::refreshRecentCaptures() >> wireshark_application.cpp:256 (Wireshark:x86_64+0x10079a36a) >>#2 WiresharkApplication::qt_static_metacall(QObject*, >> QMetaObject::Call, int, void**) >> moc_wireshark_application_Debug.cpp:234 (Wireshark:x86_64+0x1000bbceb) >>#3 QMetaObject::activate(QObject*, int, int, void**) >> :8575712 (QtCore:x86_64+0x2194ba) >>#4 main wireshark-qt.cpp:852 (Wireshark:x86_64+0x100037b0c) >> >> Thread T18 (tid=17501933, running) created by main thread at: >>#0 pthread_create :8575760 >> (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x2a34d) >>#1 QThread::start(QThread::Priority) :8575760 >> (QtCore:x86_64+0x2b5cb) >>#2 main wireshark-qt.cpp:667 (Wireshark:x86_64+0x100036bcd) >> >> SUMMARY: ThreadSanitizer: data race qstring.h:906 in >> QString::QString(QString const&) >> == >> >> >> -- >> Stig Bjørlykke > > ___ > Sent via:Wireshark-dev mailing list > Archives:https://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe > ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] ThreadSanitizer issue in RecentFileStatus()
Hi, Good question. Just spitballing here, in what thread are the constructors running? RecentFileStatus::filename_ is being set during construction, but is run() already called from the other thread? Jaap > On 2 Feb 2018, at 15:04, Stig Bjørlykke wrote: > > When running with ThreadSanitizer I constantly get this race condition > report from a QString in RecentFileStatus. Anyone else seen this? I > can't figure out the issue... > > == > WARNING: ThreadSanitizer: data race (pid=41949) > Read of size 8 at 0x7b0c000c0dd0 by thread T18: > * #0 QString::QString(QString const&) qstring.h:906 > (Wireshark:x86_64+0x100039756) >#1 QString::QString(QString const&) qstring.h:907 > (Wireshark:x86_64+0x100038998) >#2 RecentFileStatus::run() recent_file_status.cpp:33 > (Wireshark:x86_64+0x1004ffbec) >#3 non-virtual thunk to RecentFileStatus::run() > recent_file_status.cpp (Wireshark:x86_64+0x1004ffd2c) >#4 :8575680 (QtCore:x86_64+0x27b6d) > > Previous write of size 8 at 0x7b0c000c0dd0 by main thread: > * #0 QString::QString(QString const&) qstring.h:906 > (Wireshark:x86_64+0x10003976d) >#1 QString::QString(QString const&) qstring.h:907 > (Wireshark:x86_64+0x100038998) >#2 RecentFileStatus::RecentFileStatus(QString, QObject*) > recent_file_status.cpp:13 (Wireshark:x86_64+0x1004ff8b3) >#3 RecentFileStatus::RecentFileStatus(QString, QObject*) > recent_file_status.cpp:14 (Wireshark:x86_64+0x1004ffac0) >#4 WiresharkApplication::refreshRecentCaptures() > wireshark_application.cpp:256 (Wireshark:x86_64+0x10079a3b6) >#5 WiresharkApplication::qt_static_metacall(QObject*, > QMetaObject::Call, int, void**) > moc_wireshark_application_Debug.cpp:234 (Wireshark:x86_64+0x1000bbceb) >#6 QMetaObject::activate(QObject*, int, int, void**) > :8575680 (QtCore:x86_64+0x2194ba) >#7 main wireshark-qt.cpp:852 (Wireshark:x86_64+0x100037b0c) > > Issue is caused by frames marked with "*". > > Location is heap block of size 48 at 0x7b0c000c0db0 allocated by main thread: >#0 operator new(unsigned long) :8575712 > (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x6ba8e) >#1 WiresharkApplication::refreshRecentCaptures() > wireshark_application.cpp:256 (Wireshark:x86_64+0x10079a36a) >#2 WiresharkApplication::qt_static_metacall(QObject*, > QMetaObject::Call, int, void**) > moc_wireshark_application_Debug.cpp:234 (Wireshark:x86_64+0x1000bbceb) >#3 QMetaObject::activate(QObject*, int, int, void**) > :8575712 (QtCore:x86_64+0x2194ba) >#4 main wireshark-qt.cpp:852 (Wireshark:x86_64+0x100037b0c) > > Thread T18 (tid=17501933, running) created by main thread at: >#0 pthread_create :8575760 > (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x2a34d) >#1 QThread::start(QThread::Priority) :8575760 (QtCore:x86_64+0x2b5cb) >#2 main wireshark-qt.cpp:667 (Wireshark:x86_64+0x100036bcd) > > SUMMARY: ThreadSanitizer: data race qstring.h:906 in > QString::QString(QString const&) > == > > > -- > Stig Bjørlykke ___ Sent via:Wireshark-dev mailing list Archives:https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe