Skip to content

Loading…

OSX support #59

Closed
wants to merge 8 commits into from

2 participants

@patricklucas

Previously: (fixed now)

IT DOESN'T WORK YET (Much progress, and doesn't break anything on Linux, but not fully working on OSX yet)

Everything is building fine, and even appears to work - the bind() and server_init() both return successfully. However, based on netstat output, nothing is actually listening on the specified port, though if I use a unix socket everything works fine.

I'm still working on it, but posting here for discussion/suggestions.

patricklucas added some commits
@patricklucas patricklucas Sendfile is different on OSX dfe0231
@patricklucas patricklucas Add extern to some vars so it compiles with clang b79d7da
@patricklucas patricklucas Need to declare extern vars ef5efd7
@patricklucas patricklucas More OSX compatibility
On OSX (BSDs?), sin_len must be set on the sockaddr_in struct. Also, the
struct must be zeroed before use. (See http://stackoverflow.com/
questions/2244409/eaddrnotavail-when-binding-127-0-0-1-on-localhost)
8c3694c
@patricklucas patricklucas Don't zero your struct after setting things
*facepalm*
9cea62e
@patricklucas

Well that was dumb. I was zeroing the struct after setting the addr and port.

@jonashaag
Owner

Thanks a lot! However, I didn't have any trouble compiling bjoern with Clang (3.0).

Could you try if the osx branch works for you? https://github.com/jonashaag/bjoern/compare/osx

Also, are you sure Unix sockets work? Why doesn't that require sin_len to be set?!

@patricklucas

Re: unix sockets:

I was able to serve a WSGI app just fine without it, though it couldn't hurt to set sun_len in the same manner as below. I'll add that.

Looking at your osx branch:

On OSX there is no sys/sendfile.h, so it needs to be conditional on __APPLE__ to include sys/socket.h instead. (Where sendfile is defined on OSX)

Otherwise, it fails to build unless I have those externs. And it's not actually a clang thing - llvm-gcc reports the same problem. Here is sample output:

/Developer/usr/bin/clang -bundle -undefined dynamic_lookup -L/opt/local/lib build/temp.macosx-10.7-x86_64-2.7/http-parser/http_parser.o build/temp.macosx-10.7-x86_64-2.7/bjoern/bjoernmodule.o build/temp.macosx-10.7-x86_64-2.7/bjoern/common.o build/temp.macosx-10.7-x86_64-2.7/bjoern/filewrapper.o build/temp.macosx-10.7-x86_64-2.7/bjoern/portable_sendfile.o build/temp.macosx-10.7-x86_64-2.7/bjoern/request.o build/temp.macosx-10.7-x86_64-2.7/bjoern/server.o build/temp.macosx-10.7-x86_64-2.7/bjoern/wsgi.o -lev -o build/lib.macosx-10.7-x86_64-2.7/bjoern.so
ld: duplicate symbol __REMOTE_ADDR in build/temp.macosx-10.7-x86_64-2.7/bjoern/common.o and build/temp.macosx-10.7-x86_64-2.7/bjoern/bjoernmodule.o for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
error: command '/Developer/usr/bin/clang' failed with exit status 1

And using llvm-gcc:

gcc -bundle -undefined dynamic_lookup -L/opt/local/lib build/temp.macosx-10.7-x86_64-2.7/http-parser/http_parser.o build/temp.macosx-10.7-x86_64-2.7/bjoern/bjoernmodule.o build/temp.macosx-10.7-x86_64-2.7/bjoern/common.o build/temp.macosx-10.7-x86_64-2.7/bjoern/filewrapper.o build/temp.macosx-10.7-x86_64-2.7/bjoern/portable_sendfile.o build/temp.macosx-10.7-x86_64-2.7/bjoern/request.o build/temp.macosx-10.7-x86_64-2.7/bjoern/server.o build/temp.macosx-10.7-x86_64-2.7/bjoern/wsgi.o -lev -o build/lib.macosx-10.7-x86_64-2.7/bjoern.so
ld: duplicate symbol __REMOTE_ADDR in build/temp.macosx-10.7-x86_64-2.7/bjoern/common.o and build/temp.macosx-10.7-x86_64-2.7/bjoern/bjoernmodule.o for architecture x86_64
collect2: ld returned 1 exit status
error: command 'gcc' failed with exit status 1

Some searching led me to try externing those vars, which worked once I defined them in their corresponding .c files.

@patricklucas

Do you want me to refactor my branch à la your osx branch's portable_sendfile?

@jonashaag
Owner

Does the latest commit fix the header issues? If not, can you please fix them based on the osx branch?

Btw are you sure we need to set sin_len? Also the zero-ing seems to not be required?

The extern thing seems to be the correct approach although I'd prefer the linker combine the duplicate symbols... can you check if there's any such option for the OSX linker? Also to be 100% sure it's a linker issue, can you try to compile bjoern with the GCC compiler (i.e. without LLVM)?

Thanks!

@patricklucas

Does the latest commit fix the header issues? If not, can you please fix them based on the osx branch?

Missing one; you mean checkout your osx branch and fix it?

Btw are you sure we need to set sin_len? Also the zero-ing seems to not be required?

Aha - for me, zeroing is actually all that is required. When developing my patch, I set sin_len first, then tried zeroing, but I just tested and zeroing only is successful. If I do not zero the struct I get EADDRNOTAVAIL from the bind.

The extern thing seems to be the correct approach although I'd prefer the linker combine the duplicate symbols... can you check if there's any such option for the OSX linker? Also to be 100% sure it's a linker issue, can you try to compile bjoern with the GCC compiler (i.e. without LLVM)?

I'll check it out.

@patricklucas

OK - citing this[1] article, it seems that often compilers assume -fcommon, while on OSX by default it seems to use -fno-common.

You can reproduce the error I'm having by adding -fno-common to the compiler args.

I can either add -fcommon to setup.py, or use my extern changes - I would argue the latter is better C style.

[1]http://www.lurklurk.org/linkers/linkers.html#dups

@jonashaag
Owner

Does the latest commit fix the header issues? If not, can you please fix them based on the osx branch?

Missing one; you mean checkout your osx branch and fix it?

yep

Btw are you sure we need to set sin_len? Also the zero-ing seems to not be required?

Aha - for me, zeroing is actually all that is required. When developing my patch, I set sin_len first, then tried zeroing, but I just tested and zeroing only is successful. If I do not zero the struct I get EADDRNOTAVAIL from the bind.

WTF. Okay I really don't know what to do here. I think we can zero out the struct on all platforms (it doesn't hurt). Can you tell me which version of OS X you are using exactly? I'm gonna try this on a friend's Mac.

Thanks a lot for your effort btw!

@jonashaag
Owner

I can either add -fcommon to setup.py, or use my extern changes - I would argue the latter is better C style.

That's true but it's a lot more boilerplate (and thus more tedious to maintain) so please go with -fcommon.

@jonashaag jonashaag closed this
@jonashaag jonashaag reopened this
@patricklucas

Does the latest commit fix the header issues? If not, can you please fix them based on the osx branch?

Missing one; you mean checkout your osx branch and fix it?

yep

Ok I'll port my changes there.

Btw are you sure we need to set sin_len? Also the zero-ing seems to not be required?

Aha - for me, zeroing is actually all that is required. When developing my patch, I set sin_len first, then tried zeroing, but I just tested and zeroing only is successful. If I do not zero the struct I get EADDRNOTAVAIL from the bind.

WTF. Okay I really don't know what to do here. I think we can zero out the struct on all platforms (it doesn't hurt). Can you tell me which version of OS X you are using exactly? I'm gonna try this on a friend's Mac.

I'm on 10.7. Also check out that stackoverflow link I posted on the initial PR - it corroborates the zero-ing thing.

I can either add -fcommon to setup.py, or use my extern changes - I would argue the latter is better C style.
That's true but it's a lot more boilerplate (and thus more tedious to maintain) so please go with -fcommon.

Will do.

@patricklucas

Ok I'm opening another pull request with the changes.

@jonashaag jonashaag added a commit that referenced this pull request
@patricklucas patricklucas OS X support
* abstract sendfile() differences
* memset-zero sockaddr_in before using
* build with -fcommon -- let the linker strip duplicate symbols

Closes #60, #59 and #19.
29da440
@jonashaag jonashaag closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 27, 2012
  1. @patricklucas
  2. @patricklucas
  3. @patricklucas

    Need to declare extern vars

    patricklucas committed
  4. @patricklucas

    More OSX compatibility

    patricklucas committed
    On OSX (BSDs?), sin_len must be set on the sockaddr_in struct. Also, the
    struct must be zeroed before use. (See http://stackoverflow.com/
    questions/2244409/eaddrnotavail-when-binding-127-0-0-1-on-localhost)
  5. @patricklucas
  6. @patricklucas
  7. @patricklucas
  8. @patricklucas
Showing with 42 additions and 5 deletions.
  1. +1 −0 bjoern/bjoernmodule.c
  2. +1 −1 bjoern/bjoernmodule.h
  3. +4 −0 bjoern/common.c
  4. +1 −1 bjoern/common.h
  5. +2 −0 bjoern/filewrapper.c
  6. +1 −1 bjoern/filewrapper.h
  7. +29 −1 bjoern/server.c
  8. +2 −0 bjoern/wsgi.c
  9. +1 −1 bjoern/wsgi.h
View
1 bjoern/bjoernmodule.c
@@ -4,6 +4,7 @@
#include "bjoernmodule.h"
#include "filewrapper.h"
+PyObject* wsgi_app;
PyDoc_STRVAR(listen_doc,
"listen(application, host, port) -> None\n\n \
View
2 bjoern/bjoernmodule.h
@@ -1 +1 @@
-PyObject* wsgi_app;
+extern PyObject* wsgi_app;
View
4 bjoern/common.c
@@ -5,6 +5,10 @@
(c >= 'A' && c <= 'F') ? (c - 'A' + 10) : NOHEX)
#define NOHEX -1
+PyObject *_REMOTE_ADDR, *_PATH_INFO, *_QUERY_STRING, *_REQUEST_METHOD, *_GET,
+ *_HTTP_CONTENT_LENGTH, *_CONTENT_LENGTH, *_HTTP_CONTENT_TYPE, *_CONTENT_TYPE,
+ *_SERVER_PROTOCOL, *_HTTP_1_1, *_HTTP_1_0, *_wsgi_input, *_close, *_empty_string;
+
size_t unquote_url_inplace(char* url, size_t len)
{
for(char *p=url, *end=url+len; url != end; ++url, ++p) {
View
2 bjoern/common.h
@@ -23,7 +23,7 @@ enum http_status { HTTP_BAD_REQUEST = 1, HTTP_LENGTH_REQUIRED, HTTP_SERVER_ERROR
size_t unquote_url_inplace(char* url, size_t len);
void (_init_common)();
-PyObject *_REMOTE_ADDR, *_PATH_INFO, *_QUERY_STRING, *_REQUEST_METHOD, *_GET,
+extern PyObject *_REMOTE_ADDR, *_PATH_INFO, *_QUERY_STRING, *_REQUEST_METHOD, *_GET,
*_HTTP_CONTENT_LENGTH, *_CONTENT_LENGTH, *_HTTP_CONTENT_TYPE, *_CONTENT_TYPE,
*_SERVER_PROTOCOL, *_HTTP_1_1, *_HTTP_1_0, *_wsgi_input, *_close, *_empty_string;
View
2 bjoern/filewrapper.c
@@ -1,5 +1,7 @@
#include "filewrapper.h"
+PyTypeObject FileWrapper_Type;
+
static PyObject*
FileWrapper_New(PyTypeObject* cls, PyObject* args, PyObject* kwargs)
{
View
2 bjoern/filewrapper.h
@@ -2,7 +2,7 @@
#define FileWrapper_CheckExact(x) ((x)->ob_type == &FileWrapper_Type)
-PyTypeObject FileWrapper_Type;
+extern PyTypeObject FileWrapper_Type;
typedef struct {
PyObject_HEAD
View
30 bjoern/server.c
@@ -6,7 +6,12 @@
#ifdef WANT_SIGINT_HANDLING
# include <sys/signal.h>
#endif
-#include <sys/sendfile.h>
+#ifdef __APPLE__
+# include <string.h>
+# include <sys/types.h>
+#else
+# include <sys/sendfile.h>
+#endif
#include <ev.h>
#include "common.h"
#include "wsgi.h"
@@ -104,6 +109,7 @@ bool server_init(const char* hostaddr, const int port)
return false;
struct sockaddr_in sockaddr;
+ memset(&sockaddr, 0, sizeof(sockaddr));
sockaddr.sin_family = PF_INET;
inet_pton(AF_INET, hostaddr, &sockaddr.sin_addr);
sockaddr.sin_port = htons(port);
@@ -329,6 +335,26 @@ send_chunk(Request* request)
#define SENDFILE_CHUNK_SIZE 16*1024
+#ifdef __APPLE__
+
+static bool
+do_sendfile(Request* request){
+ off_t len = SENDFILE_CHUNK_SIZE;
+
+ int r = sendfile(
+ request->current_chunk_p,
+ request->client_fd,
+ 0,
+ &len,
+ NULL,
+ 0);
+ if(r == -1)
+ return handle_nonzero_errno(request);
+ return len !=0;
+}
+
+#else
+
static bool
do_sendfile(Request* request)
{
@@ -342,6 +368,8 @@ do_sendfile(Request* request)
return bytes_sent != 0;
}
+#endif
+
static bool
handle_nonzero_errno(Request* request)
{
View
2 bjoern/wsgi.c
@@ -3,6 +3,8 @@
#include "filewrapper.h"
#include "wsgi.h"
+PyTypeObject StartResponse_Type;
+
static PyObject* (start_response)(PyObject* self, PyObject* args, PyObject *kwargs);
static size_t wsgi_getheaders(Request*, PyObject* buf);
static inline bool inspect_headers(Request*);
View
2 bjoern/wsgi.h
@@ -5,4 +5,4 @@ bool wsgi_call_application(Request*);
PyObject* wsgi_iterable_get_next_chunk(Request*);
PyObject* wrap_http_chunk_cruft_around(PyObject* chunk);
-PyTypeObject StartResponse_Type;
+extern PyTypeObject StartResponse_Type;
Something went wrong with that request. Please try again.