Re: [Wireshark-dev] ThreadSanitizer issue in RecentFileStatus()

2018-02-03 Thread Gerald Combs 
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()

2018-02-03 Thread Stig Bjørlykke 
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()

2018-02-02 Thread Roland Knall 
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()

2018-02-02 Thread Gerald Combs 
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