search

openMSX / wxcatapult

23 stars 4 forks source link

[Bug] wxCatapult: x64 wxCriticalSection::Enter() crash trace [sf#488] #21

Closed openMSX-import closed 9 years ago

openMSX-import commented 9 years ago

Reported by joxy on 2013-08-01 05:37 UTC

(14:30:38) Vampier: ntdll.dll!000007fa7de31069()    Unknown
(14:30:39) Vampier: >    catapult.exe!wxCriticalSection::Enter() Line 171    C++
(14:30:39) Vampier:      catapult.exe!wxEvtHandler::AddPendingEvent(wxEvent & event) Line 1144    C++
(14:30:39) Vampier:      catapult.exe!wxPostEvent(wxEvtHandler * dest, wxEvent & event) Line 2571    C++
(14:30:39) Vampier:      catapult.exe!PipeReadThread::Entry() Line 55    C++
(14:30:39) Vampier:      catapult.exe!wxThreadInternal::DoThreadStart(wxThread * thread) Line 525    C++
(14:30:39) Vampier:      catapult.exe!wxThreadInternal::WinThreadStart(void * param) Line 552    C++
(14:30:40) Vampier:      catapult.exe!_callthreadstartex() Line 354    C
(14:30:41) Vampier:      catapult.exe!_threadstartex(void * ptd) Line 337    C
(14:30:41) Vampier:      kernel32.dll!000007fa7cfc1832()    Unknown
(14:30:42) Vampier:      ntdll.dll!000007fa7de8d609()    Unknown
(14:30:56) Vampier: CRASH!
(14:32:33) egp_: Vampier: is this x64 build?
(14:33:39) Vampier: yep
openMSX-import commented 9 years ago

Commented by joxy on 2013-08-02 04:52 UTC No. From the wx docu http://docs.wxwidgets.org/2.8.12/wx_wxevthandler.html#wxevthandleraddpendingevent :

wxEvtHandler::AddPendingEvent

void AddPendingEvent(wxEvent& event)

"This is also the method to call for inter-thread communication---it will post events safely between different threads which means that this method is thread-safe by using critical sections where needed. In a multi-threaded program, you often need to inform the main GUI thread about the status of other working threads and such notification should be done using this method."

openMSX-import commented 9 years ago

Updated by joxy on 2013-08-02 05:57 UTC

Diff:


--- old
+++ new
@@ -13,12 +13,4 @@
 (14:30:56) Vampier: CRASH!
 (14:32:33) egp_: Vampier: is this x64 build?
 (14:33:39) Vampier: yep
-(14:33:58) egp_: well it seems it adds gui events in the non-GUI thread. 
-This is ridiculously wrong
-(14:33:59) Vampier: Program: ...\derived\x64-VC-Unicode Debug\install\catapult.exe
-(14:33:59) Vampier: File: ..\..\src\openMSXController.cpp
-(14:33:59) Vampier: Line: 730
-(14:34:32) egp_: one has to use a queue to transmit events from another thread to the GUI thread.
-(14:35:13) egp_: Only the very novices at GUI programming do this mistake
-(14:35:51) egp_: This must also be fixed for x32 version.

- **Priority**: 1 --> 5
openMSX-import commented 9 years ago

Commented by joxy on 2013-08-02 06:26 UTC

// wx/thread.h

#if !defined(__WXMSW__) && !defined(__WXMAC__)
    #define wxCRITSECT_IS_MUTEX 1

    #define wxCRITSECT_INLINE inline
#else // MSW
    #define wxCRITSECT_IS_MUTEX 0

    #define wxCRITSECT_INLINE
#endif // MSW/!MSW

...

class WXDLLIMPEXP_BASE wxCriticalSection
{
...
    // enter the section (the same as locking a mutex)
    wxCRITSECT_INLINE void Enter();
...

// src/msw/thread.cpp

void wxCriticalSection::Enter()
{
    ::EnterCriticalSection((CRITICAL_SECTION *)m_buffer); //line 170
} //line 171

NB: wx seems to at least partially support x64 as there are "#ifdef WIN64" stuffs.

openMSX-import commented 9 years ago

Updated by joxy on 2013-08-02 06:34 UTC

openMSX-import commented 9 years ago

Commented by manuelbi on 2013-08-05 21:04 UTC Let's not dive into the code of wxWidgets and assume it's not (very) buggy...

openMSX-import commented 9 years ago

Updated by manuelbi on 2014-11-23 20:18 UTC

openMSX-import commented 9 years ago

Commented by manuelbi on 2014-11-23 20:18 UTC As there's no way to reproduce this (none is described), let's simply close this bug.