You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by andrey-agenosov <gi...@git.apache.org> on 2014/03/25 22:53:25 UTC
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
GitHub user andrey-agenosov opened a pull request:
https://github.com/apache/thrift/pull/86
fix issue with cpp server on Windows (WSAStartup wasn't called)
Hello!
I tried to launch C++ server on Windows OS and found a bug - TServerSocket can't start to listen because WSAStartup wasn't called.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/andrey-agenosov/thrift master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/86.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #86
----
commit cd32373f6deca00fc5863739131ad6f45be82a3c
Author: unknown <an...@gmail.com>
Date: 2014-03-25T21:48:40Z
fix issue with cpp server on Windows (WSAStartup wasn't called)
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by ben-craig <gi...@git.apache.org>.
Github user ben-craig commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-39333221
On Windows, you should call TWinsockSingleton::create() at an opportune time in your code. This can be found in the thrift/windows/TWinsockSingleton.h header.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by bufferoverflow <gi...@git.apache.org>.
Github user bufferoverflow commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-50206649
please close this
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by chris5287 <gi...@git.apache.org>.
Github user chris5287 commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-39324600
Create https://issues.apache.org/jira/browse/THRIFT-2442 for this pull request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by pascal-bach <gi...@git.apache.org>.
Github user pascal-bach commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-39438142
I tried the above patch in my application using multiple servers and I didn't notice any issues with calling `TWinsockSingleton::create()` multiple times. Even leaving the old code that calls `TWinsockSingleton::create()` while initializing the application in causes no problem.
I would vote to go with option 2 as it makes the usage of thrift easier.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by jfarrell <gi...@git.apache.org>.
Github user jfarrell closed the pull request at:
https://github.com/apache/thrift/pull/86
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by henrique <gi...@git.apache.org>.
Github user henrique commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-39362555
Would this work if you have multiple servers simultaneously opened in the same process?
I also don't like the idea of having to start it manually, specially because it's a windows only requirement.
It would be really nice to have it automatically started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by uwydoc <gi...@git.apache.org>.
Github user uwydoc commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-47415698
This patch should work. Anyway, calling `TWinsockSingleton::create()` with `ifdef` is what `TSocket::local_open` is doing for client winsock2 initialization.
As for the multiple-servers concern proposed by @henrique , `TWinsockSingleton` uses `boost::call_once` to ensure thread-safe one-time initialization, so no worry.
As to the build error, after a simple glance at the build log:
```
...
$ sudo apt-get install -qq php5-dev php5-cli phpunit
Failed to fetch http://security.ubuntu.com/ubuntu/pool/main/p/php5/php5-common_5.3.10-1ubuntu3.9_amd64.deb 404 Not Found [IP: 2001:67c:1360:8c01::19 80]
Failed to fetch http://security.ubuntu.com/ubuntu/pool/main/p/php5/php5-cli_5.3.10-1ubuntu3.9_amd64.deb 404 Not Found [IP: 2001:67c:1360:8c01::19 80]
Failed to fetch http://security.ubuntu.com/ubuntu/pool/main/p/php5/php-pear_5.3.10-1ubuntu3.9_all.deb 404 Not Found [IP: 2001:67c:1360:8c01::19 80]
Failed to fetch http://security.ubuntu.com/ubuntu/pool/main/p/php5/php5-dev_5.3.10-1ubuntu3.9_amd64.deb 404 Not Found [IP: 2001:67c:1360:8c01::19 80]
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
[33;1mThe command "sudo apt-get install -qq php5-dev php5-cli phpunit" failed and exited with 100 during before_install.[0m
Your build has been stopped.
```
it is clear that the error had resulted from some network error with the build machine, not the patch itself.
So i think the patch owner @andrey-agenosov could submit again, or some master can manually merge this pull request(or maybe fix the build error).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by pascal-bach <gi...@git.apache.org>.
Github user pascal-bach commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-39363924
I'm using multiple servers running in seperate threads and I call the create method only once in my application.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: fix issue with cpp server on Windows (WSAStar...
Posted by ben-craig <gi...@git.apache.org>.
Github user ben-craig commented on the pull request:
https://github.com/apache/thrift/pull/86#issuecomment-39373713
This should go in one of three directions:
Option 1: completely abandon this path
Option 2: Be more thorough in calling TWinsockSingleton::create(). You're calling it for serverSocket::listen, but you aren't doing anything on client creation. I don't particularly care for this approach, because it adds overhead that isn't strictly required.
Option 3: Make a platform abstraction for TWinsockSingleton::create() that can be called on all OSes. It would be a no op on posix platforms, but at least then you don't need ifdefs in your code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---