Skip to content

Commit

Permalink
Fix #636, FD leak for requesting empty HTTP stream. 2.0.241
Browse files Browse the repository at this point in the history
  • Loading branch information
winlinvip committed Apr 30, 2017
1 parent ff87318 commit ae54501
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 2 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ Remark:

## History

* v2.0, 2017-04-30, Fix [#636][bug #636], FD leak for requesting empty HTTP stream. 2.0.241
* v2.0, 2017-04-23, Fix [#851][bug #851], HTTP API support number of video frames for FPS. 2.0.240
* <strong>v2.0, 2017-04-18, [2.0 release1(2.0.239)][r2.0r1] released. 86515 lines.</strong>
* v2.0, 2017-04-18, Fix [#848][bug #848], crash at HTTP fast buffer grow. 2.0.239
Expand Down Expand Up @@ -1293,6 +1294,7 @@ Winlin
[bug #844]: https://github.com/ossrs/srs/issues/844
[bug #848]: https://github.com/ossrs/srs/issues/848
[bug #851]: https://github.com/ossrs/srs/issues/851
[bug #636]: https://github.com/ossrs/srs/issues/636
[bug #xxxxxxxxxx]: https://github.com/ossrs/srs/issues/xxxxxxxxxx

[exo #828]: https://github.com/google/ExoPlayer/pull/828
Expand Down
13 changes: 13 additions & 0 deletions trunk/src/app/srs_app_http_conn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1310,6 +1310,19 @@ SrsResponseOnlyHttpConn::~SrsResponseOnlyHttpConn()
{
}

int SrsResponseOnlyHttpConn::pop_message(ISrsHttpMessage** preq)
{
int ret = ERROR_SUCCESS;

SrsStSocket skt(stfd);

if ((ret = parser->parse_message(&skt, this, preq)) != ERROR_SUCCESS) {
return ret;
}

return ret;
}

int SrsResponseOnlyHttpConn::on_got_http_message(ISrsHttpMessage* msg)
{
int ret = ERROR_SUCCESS;
Expand Down
9 changes: 8 additions & 1 deletion trunk/src/app/srs_app_http_conn.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ class SrsHttpUri

class SrsHttpConn : public SrsConnection
{
private:
protected:
SrsHttpParser* parser;
ISrsHttpServeMux* http_mux;
public:
Expand Down Expand Up @@ -421,6 +421,13 @@ class SrsResponseOnlyHttpConn : public SrsHttpConn
public:
SrsResponseOnlyHttpConn(IConnectionManager* cm, st_netfd_t fd, ISrsHttpServeMux* m);
virtual ~SrsResponseOnlyHttpConn();
public:
// Directly read a HTTP request message.
// It's exported for HTTP stream, such as HTTP FLV, only need to write to client when
// serving it, but we need to start a thread to read message to detect whether FD is closed.
// @see https://github.com/ossrs/srs/issues/636#issuecomment-298208427
// @remark Should only used in HTTP-FLV streaming connection.
virtual int pop_message(ISrsHttpMessage** preq);
public:
virtual int on_got_http_message(ISrsHttpMessage* msg);
};
Expand Down
19 changes: 19 additions & 0 deletions trunk/src/app/srs_app_http_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ using namespace std;
#include <srs_app_pithy_print.hpp>
#include <srs_app_source.hpp>
#include <srs_app_server.hpp>
#include <srs_app_recv_thread.hpp>

#endif

Expand Down Expand Up @@ -535,10 +536,28 @@ int SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r)
#ifdef SRS_PERF_FAST_FLV_ENCODER
SrsFastFlvStreamEncoder* ffe = dynamic_cast<SrsFastFlvStreamEncoder*>(enc);
#endif

// Use receive thread to accept the close event to avoid FD leak.
// @see https://github.com/ossrs/srs/issues/636#issuecomment-298208427
SrsHttpMessage* hr = dynamic_cast<SrsHttpMessage*>(r);
SrsResponseOnlyHttpConn* hc = dynamic_cast<SrsResponseOnlyHttpConn*>(hr->connection());

SrsHttpRecvThread* trd = new SrsHttpRecvThread(hc);
SrsAutoFree(SrsHttpRecvThread, trd);

if ((ret = trd->start()) != ERROR_SUCCESS) {
srs_error("http: start notify thread failed, ret=%d", ret);
return ret;
}

// TODO: free and erase the disabled entry after all related connections is closed.
while (entry->enabled) {
pprint->elapse();

// Whether client closed the FD.
if ((ret = trd->error_code()) != ERROR_SUCCESS) {
return ret;
}

// get messages from consumer.
// each msg in msgs.msgs must be free, for the SrsMessageArray never free them.
Expand Down
41 changes: 41 additions & 0 deletions trunk/src/app/srs_app_recv_thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include <srs_core_performance.hpp>
#include <srs_app_config.hpp>
#include <srs_app_source.hpp>
#include <srs_app_http_conn.hpp>
#include <srs_core_autofree.hpp>

using namespace std;

Expand Down Expand Up @@ -523,3 +525,42 @@ void SrsPublishRecvThread::set_socket_buffer(int sleep_ms)
rtmp->set_recv_buffer(nb_rbuf);
}

SrsHttpRecvThread::SrsHttpRecvThread(SrsResponseOnlyHttpConn* c)
{
conn = c;
error = ERROR_SUCCESS;
trd = new SrsOneCycleThread("http-receive", this);
}

SrsHttpRecvThread::~SrsHttpRecvThread()
{
srs_freep(trd);
}

int SrsHttpRecvThread::start()
{
return trd->start();
}

int SrsHttpRecvThread::error_code()
{
return error;
}

int SrsHttpRecvThread::cycle()
{
int ret = ERROR_SUCCESS;

while (true) {
ISrsHttpMessage* req = NULL;
SrsAutoFree(ISrsHttpMessage, req);

if ((ret = conn->pop_message(&req)) != ERROR_SUCCESS) {
error = ret;
break;
}
}

return ret;
}

26 changes: 26 additions & 0 deletions trunk/src/app/srs_app_recv_thread.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ class SrsRtmpConn;
class SrsSource;
class SrsRequest;
class SrsConsumer;
class SrsHttpConn;
class SrsResponseOnlyHttpConn;

/**
* for the recv thread to handle the message.
Expand Down Expand Up @@ -215,5 +217,29 @@ class SrsPublishRecvThread : virtual public ISrsMessageHandler
virtual void set_socket_buffer(int sleep_ms);
};

/**
* The HTTP receive thread, try to read messages util EOF.
* For example, the HTTP FLV serving thread will use the receive thread to break
* when client closed the request, to avoid FD leak.
* @see https://github.com/ossrs/srs/issues/636#issuecomment-298208427
*/
class SrsHttpRecvThread : public ISrsOneCycleThreadHandler
{
private:
SrsResponseOnlyHttpConn* conn;
SrsOneCycleThread* trd;
int error;
public:
SrsHttpRecvThread(SrsResponseOnlyHttpConn* c);
virtual ~SrsHttpRecvThread();
public:
virtual int start();
public:
virtual int error_code();
// interface ISrsOneCycleThreadHandler
public:
virtual int cycle();
};

#endif

2 changes: 1 addition & 1 deletion trunk/src/core/srs_core.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
// current release version
#define VERSION_MAJOR 2
#define VERSION_MINOR 0
#define VERSION_REVISION 240
#define VERSION_REVISION 241

// generated by configure, only macros.
#include <srs_auto_headers.hpp>
Expand Down

1 comment on commit ae54501

@winlinvip
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by f2b4bc7

Please sign in to comment.