[WIP] Make FastCGI work on OS X #2386
Conversation
| @@ -104,7 +104,9 @@ void ThreadInfo::onSessionInit() { | |||
| m_stacklimit = t_stackbase - (rl.rlim_cur - StackSlack); | |||
| } else { | |||
| m_stacklimit = (char *)s_stackLimit + StackSlack; | |||
| #ifndef __APPLE__ | |||
There was a problem hiding this comment.
This needs a comment and a TODO.
There was a problem hiding this comment.
This shouldn't have been included in this diff. It's something that's broken in debug builds on OSX that I haven't figured out yet.
There was a problem hiding this comment.
It has something to do with the pthread APIs returning different answers for stack size. Probably a quirk of Mach, but yeah, when I looked at JIT support I didn't end up spending enough time on it to figure out what we should be doing here.
|
It might be better to split the thrift changes into a separate pull assuming they'll independently compile. (This would mean an HHVM change pull, a thrift change pull, and then a final config change pull to add register_fastcgi_server, I think.) This way the first set doesn't get slowed down on the thrift changes [plus I think we'd want the ability to revert all three of those things independently.] |
When constructing static objects in OSX, the constructor initialization list is evaluated. This means a String object is constructed when the MemoryManager has not been initialized yet (in init_thread_locals).
| @@ -106,7 +106,7 @@ class Extension : public IDebuggable { | |||
| // this module was built against. | |||
| int64_t m_hhvmAPIVersion; | |||
|
|
|||
| const String m_name; | |||
| std::string m_name; | |||
There was a problem hiding this comment.
Yeah, this bothers me too.
It's not technically wrong, by any measure, but why?
|
Needs to be rebased for @ptarjan's third_party move. Generally though this is going to take a little bit longer to merge since I'd like to separately upstreat the thrift/proxygen changes to keep the forks in sync. |
|
@danslo Hi. I am assuming this PR still in a holding pattern? Or is it obsolete now? Do you want us to keep it open? Just doing a bit of triaging. |
|
Hey Joel, like Sara mentioned, this will need to be rebased and some of the changes need to be moved to hhvm/third-party or even fixed further upstream and added as submodules there. I've been traveling so I haven't had time to look at this yet. Do intend to update this PR in the future. |
|
Closing until this is actionable. |
The thrift/proxygen patches are not done in their own repos due to:
At some point we'll have to try to land these elsewhere and use them as
a submodule. Today is not the time.