Opened 9 years ago

Closed 3 years ago

#2427 closed defect (fixed)

[PATCH] AtlasUI does not open on on commandline Mavericks 10.9

Reported by: Echelon9 Owned by: trompetin17
Priority: Must Have Milestone: Alpha 24
Component: Atlas editor Keywords: patch macos
Cc: Patch: Phab:D2788

Description (last modified by wraitii)

Running AtlasUI from commandline results in the following failure inside FSEventStreamCreate (CarbonCore.framework), called within CreateEventStream( DirWatchMap path ).

$ ./pyrogenesis -editor
TIMER| LoadDLL: 6.15073 s
2014-02-08 15:00 pyrogenesis[65341] (CarbonCore.framework) FSEventStreamCreate: _FSEventStreamCreate: ERROR: could not allocate 0 bytes for array of path strings
2014-02-08 15:00 pyrogenesis[65341] (CarbonCore.framework) FSEventStreamScheduleWithRunLoop(): failed assertion 'streamRef != NULL'

2014-02-08 15:00 pyrogenesis[65341] (CarbonCore.framework) FSEventStreamStart(): failed assertion 'streamRef != NULL'

dir_watch.cpp(143): Assertion failed: "0 && (L"event_loop FSEventStreamStart failed!")"
Segmentation fault: 11

It appears parameter pathsToWatch is empty at the time, as path.size() is zero in CreateEventStream( DirWatchMap path ).

Attachments (7)

test.png (849.2 KB ) - added by trompetin17 8 years ago.
2427.patch (3.3 KB ) - added by trompetin17 8 years ago.
FIXED
2427.2.patch (3.0 KB ) - added by trompetin17 8 years ago.
LAST ONE
2427.3.patch (1.6 KB ) - added by trompetin17 8 years ago.
2427.4.patch (2.2 KB ) - added by trompetin17 8 years ago.
2427.5.patch (2.0 KB ) - added by trompetin17 8 years ago.
2427_dic.diff (2.8 KB ) - added by trompetin17 8 years ago.

Download all attachments as: .zip

Change History (43)

comment:1 by Echelon9, 9 years ago

Instrumenting dir_watch_Add(m_path, m_watch), which is the OS X specific implementation of the wrapper function RealDirectory::Watch() indicates it is never called prior to entering FSEventStreamCreate().

Hence why DirWatchMap path.size() equals zero.

comment:2 by Echelon9, 9 years ago

In fact, RealDirectory::Watch() is never called with ./pyrogenesis -editor, although it is called with commandline invocation of ./pyrogenesis (on directory "/").

This error appears in part due to InitVfs() explicitly not being called on the AtlasUI path. The code assumes Atlas handles this itself.

Last edited 9 years ago by Echelon9 (previous) (diff)

comment:3 by leper, 9 years ago

Milestone: Alpha 16Alpha 17

comment:4 by historic_bruno, 8 years ago

The reported crash is certainly a bug, CreateEventStream shouldn't crash just because an empty vector is passed in. Though I'm not sure if this will ever happen in normal conditions.

Now that I'm able to test on Mavericks again, it seems the issues with Atlas are more complex. After fixing the one crash, there is another in Atlas' message passing system (this one occurs when starting from the main menu too). In fact, InitVfs is supposed to be run with other init code in response to an Init message, but that never happens. It crashes at GameLoop.cpp:165:

* thread #6: tid = 0x37d92, 0x0000000100908a30 pyrogenesis_dbg`MessagePasserImpl::Retrieve() [inlined] std::__1::deque<AtlasMessage::IMessage*, std::__1::allocator<AtlasMessage::IMessage*> >::front(this=0x00007fff5fbfd348) + 23 at deque:1733, stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
  * frame #0: 0x0000000100908a30 pyrogenesis_dbg`MessagePasserImpl::Retrieve() [inlined] std::__1::deque<AtlasMessage::IMessage*, std::__1::allocator<AtlasMessage::IMessage*> >::front(this=0x00007fff5fbfd348) + 23 at deque:1733
    frame #1: 0x0000000100908a19 pyrogenesis_dbg`MessagePasserImpl::Retrieve() [inlined] std::__1::queue<AtlasMessage::IMessage*, std::__1::deque<AtlasMessage::IMessage*, std::__1::allocator<AtlasMessage::IMessage*> > >::front(this=0x00007fff5fbfd348) at queue:277
    frame #2: 0x0000000100908a19 pyrogenesis_dbg`MessagePasserImpl::Retrieve(this=0x00007fff5fbfd2e0) + 201 at MessagePasserImpl.cpp:107
    frame #3: 0x0000000100904b1b pyrogenesis_dbg`RunEngine(data=0x00007fff5fbfeab8) + 1035 at GameLoop.cpp:165
    frame #4: 0x00007fff92a4d899 libsystem_pthread.dylib`_pthread_body + 138
    frame #5: 0x00007fff92a4d72a libsystem_pthread.dylib`_pthread_start + 137

That's admittedly hard to know without a debug build, which is also currently broken in the same build environment.

It's very reproducible for me on Mavericks 10.9.4 building w/ libc++ and Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn). I might try switching back to libstdc++ to see if it makes any difference.

Last edited 8 years ago by historic_bruno (previous) (diff)

comment:5 by historic_bruno, 8 years ago

Priority: Should HaveMust Have

This error may be relevant:

2014-09-07 17:02:12.075 pyrogenesis_dbg[24045:507] -[NSApplication transformToForegroundApplication]: unrecognized selector sent to instance 0x7f9cb0d088f0

comment:6 by historic_bruno, 8 years ago

That error is quite relevant after all :)

#2650 duplicated this, but according to wraitii, moving to SDL2 fixes the root cause of the problem. I will still commit a fix for the partly related empty paths crash, then focus on SDL2 for OS X.

in reply to:  5 comment:7 by trompetin17, 8 years ago

FIXED, please review my patch and my image Replying to historic_bruno:

This error may be relevant:

2014-09-07 17:02:12.075 pyrogenesis_dbg[24045:507] -[NSApplication transformToForegroundApplication]: unrecognized selector sent to instance 0x7f9cb0d088f0
Last edited 8 years ago by historic_bruno (previous) (diff)

by trompetin17, 8 years ago

Attachment: test.png added

comment:8 by trompetin17, 8 years ago

Keywords: review patch added
Owner: set to trompetin17
Summary: AtlasUI does not open on on commandline Mavericks 10.9[PATCH] AtlasUI does not open on on commandline Mavericks 10.9

comment:9 by Stan`, 8 years ago

Working fine on windows 8.1.1 here (Compilation and Game, and ATlas)

by trompetin17, 8 years ago

Attachment: 2427.patch added

FIXED

by trompetin17, 8 years ago

Attachment: 2427.2.patch added

LAST ONE

by trompetin17, 8 years ago

Attachment: 2427.3.patch added

by trompetin17, 8 years ago

Attachment: 2427.4.patch added

by trompetin17, 8 years ago

Attachment: 2427.5.patch added

comment:10 by historic_bruno, 8 years ago

The usage and comment in the patch don't agree with the wxWidgets documentation for OSXIsGUIApplication:

This method is called during the application startup and returns true by default. In this case, wxWidgets ensures that the application is ran as a foreground, GUI application so that the user can interact with it normally, even if it is not bundled. If this is undesired, i.e. if the application doesn't need to be brought to the foreground, this method can be overridden to return false.

You can see what it does here: http://trac.wxwidgets.org/browser/wxWidgets/trunk/src/osx/cocoa/utils.mm

In other words, it is only intended to override the default foreground behavior for non-bundled builds, and wxWidgets should handle bundled and non-bundled apps automatically. I've tested after building as a bundle, but the same errors occur.

The interesting thing is that overriding OSXIsGUIApplication to return false will eliminate the call to transformToForegroundApplication. I'm not convinced that's the problem though, maybe just a symptom of something else failing badly during init.

comment:11 by historic_bruno, 8 years ago

In the second patch, the changes to ParamNode.cpp (presumably to fix the debug build) cause the test_overlay_tokens and test_overlay_remove_nonexistent_token tests to fail. Tested on Windows with MSVC 2010 and Boost 1.53.

Last edited 8 years ago by historic_bruno (previous) (diff)

comment:12 by historic_bruno, 8 years ago

Ah, that was already committed by leper in r15735 and r15737.

comment:13 by historic_bruno, 8 years ago

I've found a bug in wxWidgets triggered by my OS X VMs: http://trac.wxwidgets.org/ticket/16555

Applying a quick fix to revert that change fixes Atlas for me, on both Lion and Mavericks. Applying a patch for dir_watch.cpp fixes some assertions that occur in the background, but even without that, Atlas manages to open and work correctly.

in reply to:  13 ; comment:14 by historic_bruno, 8 years ago

Replying to historic_bruno:

I've found a bug in wxWidgets triggered by my OS X VMs: http://trac.wxwidgets.org/ticket/16555

I uploaded a patch there, if anyone on OS X cares to help test ;)

comment:15 by ben, 8 years ago

In 15748:

Fixes crash on OS X if dir_watch_Poll is called during init before dir_watch_Add, refs #2427, #2650

in reply to:  14 comment:16 by historic_bruno, 8 years ago

Replying to historic_bruno:

Replying to historic_bruno:

I've found a bug in wxWidgets triggered by my OS X VMs: http://trac.wxwidgets.org/ticket/16555

I uploaded a patch there, if anyone on OS X cares to help test ;)

It has been committed in r77699 :)

comment:17 by trompetin17, 8 years ago

Hi historic_bruno, Did you see my comment in my last patch why I did that in DLLInterface.cpp?

in reply to:  17 comment:18 by historic_bruno, 8 years ago

Replying to trompetin17:

Hi historic_bruno, Did you see my comment in my last patch why I did that in DLLInterface.cpp?

Yes, I was responding to the comments here :)

comment:19 by Echelon9, 8 years ago

I can confirm that 2427.5.patch against trunk (r15812) allows AltasUI to open via the in-game menu where previously it would hard crash for me.

Running OS X 10.9.5 and now using SDL2 per recent SVN commits.

Thanks for the patch trompetin17.

comment:20 by historic_bruno, 8 years ago

I think the problem starting Atlas from the in-game menu is due to SDL having a similar init process to wxWidgets. There is an SDLApplication subclass of NSApplication, like the wxNSApplication subclass for wxWidgets. Calling the sharedApplication method the first time initializes a global instance of that class, after that it returns a reference to it. It seems the instance is still active after the engine restarts and wxWidgets tries to do its own init process, but it won't init an wxNSApplication, only return a reference to the existing SDLApplication.

trompetin17's patch should work around that by not calling OnInitGui() and the use of wxEntryStart will avoid some other init code as well, I'm not certain on the other effects of this. It would be ideal if we could shutdown the global SDLApplication and then do normal wxWidgets init.

Last edited 8 years ago by historic_bruno (previous) (diff)

comment:21 by ben, 8 years ago

In 15821:

Fixes starting Atlas from in-game on OS X, patch by trompetin17, refs #2427

comment:22 by historic_bruno, 8 years ago

I committed a modified version of the patch that only applies to OS X. Even with the patch, I get an error in the terminal:

2014-09-28 00:12:29.679 pyrogenesis[15066:507] _createMenuRef called with existing principal MenuRef already associated with menu
2014-09-28 00:12:29.705 pyrogenesis[15066:507] (
	0   CoreFoundation                      0x00007fff91fb325c __exceptionPreprocess + 172
	1   libobjc.A.dylib                     0x00007fff93c8fe75 objc_exception_throw + 43
	2   CoreFoundation                      0x00007fff91fb310c +[NSException raise:format:] + 204
	3   AppKit                              0x00007fff959cacf4 -[NSCarbonMenuImpl _createMenuRef] + 62
	4   AppKit                              0x00007fff959ca6ca -[NSCarbonMenuImpl _instantiateCarbonMenu] + 143
	5   AppKit                              0x00007fff959c8e92 -[NSApplication finishLaunching] + 876
	6   AppKit                              0x00007fff959c87f3 -[NSApplication run] + 128
	7   libAtlasUI.dylib                    0x000000013229353e _ZN14wxGUIEventLoop8OSXDoRunEv + 110
	8   libAtlasUI.dylib                    0x00000001324538e4 _ZN13wxCFEventLoop5DoRunEv + 52
	9   libAtlasUI.dylib                    0x00000001323d92e5 _ZN15wxEventLoopBase3RunEv + 165
	10  libAtlasUI.dylib                    0x00000001323a7ed1 _ZN16wxAppConsoleBase8MainLoopEv + 209
	11  libAtlasUI.dylib                    0x000000013225a11a _ZN5wxApp5OnRunEv + 26
	12  libAtlasUI.dylib                    0x0000000131fba024 Atlas_StartWindow + 164
	13  pyrogenesis                         0x000000010519147b _Z10BeginAtlasRK11CmdLineArgsRK9DllLoader + 443
	14  pyrogenesis                         0x000000010506cfa5 _Z20ATLAS_RunIfOnCmdLineRK11CmdLineArgsb + 101
	15  pyrogenesis                         0x0000000104ef3a70 _ZL14RunGameOrAtlasiPPKc + 5360
	16  pyrogenesis                         0x0000000104ef24e5 main + 53
	17  libdyld.dylib                       0x00007fff942995fd start + 1
	18  ???                                 0x0000000000000001 0x0 + 1
)

It is probably going to be buggy as explained above, so I'm not closing the ticket yet until this can be discussed further.

comment:23 by Echelon9, 8 years ago

Yes, I also saw the above error in comment 22. At least it's not a crash!

in reply to:  20 comment:24 by trompetin17, 8 years ago

Hi historicbruno, I did some changes and it worked in my environment,

    wxApp::SetInstance(new AtlasDLLApp());
    wxEntry(argc, argv);
    
    /*
     wxEntryStart(argc, argv);
     wxTheApp->OnInit();
     wxTheApp->OnRun();
     wxTheApp->OnExit();
     wxEntryCleanup();
     */

I had to move the method Atlas_StartWindow at last because i was getting an error "Allocation of Imcomplete type 'AtlasDLLApp'"

HistoricBruno, tell me what should i do now?

Replying to historic_bruno:

I think the problem starting Atlas from the in-game menu is due to SDL having a similar init process to wxWidgets. There is an SDLApplication subclass of NSApplication, like the wxNSApplication subclass for wxWidgets. Calling the sharedApplication method the first time initializes a global instance of that class, after that it returns a reference to it. It seems the instance is still active after the engine restarts and wxWidgets tries to do its own init process, but it won't init an wxNSApplication, only return a reference to the existing SDLApplication.

trompetin17's patch should work around that by not calling OnInitGui() and the use of wxEntryStart will avoid some other init code as well, I'm not certain on the other effects of this. It would be ideal if we could shutdown the global SDLApplication and then do normal wxWidgets init.

comment:25 by historic_bruno, 8 years ago

Keywords: review removed
Milestone: Alpha 17Alpha 18

We can work on this more for A18, I think it works well enough for A17.

by trompetin17, 8 years ago

Attachment: 2427_dic.diff added

comment:26 by trompetin17, 8 years ago

Keywords: review added

Hi, HistoricBruno Could you review this one. I tested on my mac and works

Last edited 8 years ago by trompetin17 (previous) (diff)

in reply to:  26 comment:27 by historic_bruno, 8 years ago

Replying to trompetin17:

Hi, HistoricBruno Could you review this one. I tested on my mac and works

Doesn't work for me on Yosemite - starting Atlas from in-game, it just hangs after the exception mentioned above.

comment:28 by Simon Georges, 8 years ago

Component: Core engineAtlas editor

comment:29 by historic_bruno, 8 years ago

Keywords: review removed

comment:30 by Stan`, 8 years ago

Milestone: Alpha 18Alpha 19

comment:31 by Stan`, 7 years ago

Milestone: Alpha 19Alpha 20

Moving it for lack of activity ;)

comment:32 by elexis, 7 years ago

Milestone: Alpha 20Backlog

Backlogging due to lack of progress.

comment:33 by wraitii, 3 years ago

Description: modified (diff)
Milestone: BacklogAlpha 24

This appears to be exactly the issue I'm running into when setting OSXIsGUIApplication to true. The reason why I'd want this is that it fixes startup in non-bundled mode, i.e. -editor mode.

From what I can tell they're using similar-but-different hacks for moving the app into the foreground. SDL uses a workaround: https://bugzilla.libsdl.org/show_bug.cgi?id=3051 (found also here ). WxWidgets appears to use another one...

That being said, I think the issue is that since we created an SDLApplication subclass of NSApplication, sharedApplication returns that (and not wxwidget's WXNsApplication), which breaks because apparently objective C can do fancy pointer checking. It seems to be fundamentally broken on some level however, because we're subclassing in two different ways that are fundamentally incompatible. I suspect the 'double-write' issue comes from there too, as we are probably getting the SDL hook instead of the wxwidgets one.

See doc: https://developer.apple.com/documentation/appkit/nsapplication?language=objc

I must say I don't really see an easier fix than properly restarting atlas...

comment:34 by wraitii, 3 years ago

Patch: Phab:D2788

See Phab:D2788 for "properly restarting atlas".

comment:35 by Krinkle, 3 years ago

Keywords: macos added

comment:36 by wraitii, 3 years ago

Resolution: fixed
Status: newclosed

In 23926:

Fix issues relating to SDL and wxWidgets interaction in Atlas.

This fixes the transfer of key inputs from WxWidgets to SDL, making it possible to type in the in-game GUI from Atlas.

Also fixes whitespace issues in some Atlas files.

The following improvements are OSX specific:

  • fixes an SDL assertion related to unused subsystems in Atlas.
  • Remove the 'osxguiapplication' override. This fixes the editor starting up in the background and not accepting input when launched from in-game.
  • To prevent an issue with sdl/wxwidgets conflict when running from inside the game, actually boot a new instance (see #2427)

Reported by: wik (Many thanks for your investigations)

Tested by: trompetin17, Stan

Fixes #2427
Fixes #2846

Differential Revision: https://code.wildfiregames.com/D2788

Note: See TracTickets for help on using tickets.