Repository: ossrs/srs
File: trunk/src/app/srs_app_server.cpp (C++)

Code before:
            resource = new SrsRtmpConn(this, stfd2, ip, port);
        } else if (listener == api_listener_ || listener == apis_listener_) {
            bool is_https = listener == apis_listener_;
            resource = new SrsHttpxConn(is_https, this, new SrsTcpConnection(stfd2), http_api_mux, ip, port);
        } else if (listener == http_listener_ || listener == https_listener_) {
            bool is_https = listener == https_listener_;
            resource = new SrsHttpxConn(is_https, this, new SrsTcpConnection(stfd2), http_server, ip, port);
#ifdef SRS_RTC
        } else if (listener == webrtc_listener_) {
            resource = new SrsRtcTcpConn(new SrsTcpConnection(stfd2), ip, port);
#endif
        } else if (listener == exporter_listener_) {
            // TODO: FIXME: Maybe should support https metrics.
            bool is_https = false;
            resource = new SrsHttpxConn(is_https, this, new SrsTcpConnection(stfd2), http_api_mux, ip, port);
        } else {
            srs_close_stfd(stfd2);
            srs_warn("Close for invalid fd=%d, ip=%s:%d", fd, ip.c_str(), port);
            return err;
        }
    }

    // For RTC TCP connection, use resource executor to manage the resource.
    SrsRtcTcpConn* raw_conn = dynamic_cast<SrsRtcTcpConn*>(resource);
    if (raw_conn) {
        SrsSharedResource<SrsRtcTcpConn>* conn = new SrsSharedResource<SrsRtcTcpConn>(raw_conn);
        SrsExecutorCoroutine* executor = new SrsExecutorCoroutine(_srs_rtc_manager, conn, raw_conn, raw_conn);
        raw_conn->setup_owner(conn, executor, executor);
        if ((err = executor->start()) != srs_success) {
            srs_freep(executor);
            return srs_error_wrap(err, "start executor");
        }
        return err;
    }

    // Use connection manager to manage all the resources.
    srs_assert(resource);
    conn_manager->add(resource);

    // If connection is a resource to start, start a coroutine to handle it.
    ISrsStartable* conn = dynamic_cast<ISrsStartable*>(resource);
    srs_assert(conn);
    if ((err = conn->start()) != srs_success) {
        return srs_error_wrap(err, "start conn coroutine");
    }

    return err;
}

srs_error_t SrsServer::on_before_connection(srs_netfd_t& stfd, const std::string& ip, int port)
{

Review comment (bug):
Be careful the `raw_conn (SrsRtcTcpConn resource)` and its `conn (SrsSharedResource wrapper)`, the `raw_conn` will be add to `conn_manager`, the tcp connection resource manager of `SrsServer`, in line 1244, and its wrapper, a SrsSharedResource, will created below in line 1233, this wrapper didn't added to any resource manager, but it will be released by `_srs_rtc_manager` in `SrsExecutorCoroutine`'s destructor. Then the `raw_conn` stored in `conn_manager` will became a wild pointer, the good news is that the `conn_manager` will never remove it, so the process will never crash, but it's still a problem, is it?

So, there are two problem: 
1. the `raw_conn` will be added to `conn_manager`, which will never remove it;
2. the `raw_conn` inside `conn_manager` will became wild pointer once its wapper, the `SrsSharedResource` released by `_srs_rtc_manager` inside `SrsExecutorCoroutine`'s  desctructor.

Code after:
            resource = new SrsRtmpConn(this, stfd2, ip, port);
        } else if (listener == api_listener_ || listener == apis_listener_) {
            bool is_https = listener == apis_listener_;
            resource = new SrsHttpxConn(is_https, this, new SrsTcpConnection(stfd2), http_api_mux, ip, port);
        } else if (listener == http_listener_ || listener == https_listener_) {
            bool is_https = listener == https_listener_;
            resource = new SrsHttpxConn(is_https, this, new SrsTcpConnection(stfd2), http_server, ip, port);
#ifdef SRS_RTC
        } else if (listener == webrtc_listener_) {
            resource = new SrsRtcTcpConn(new SrsTcpConnection(stfd2), ip, port);
#endif
        } else if (listener == exporter_listener_) {
            // TODO: FIXME: Maybe should support https metrics.
            bool is_https = false;
            resource = new SrsHttpxConn(is_https, this, new SrsTcpConnection(stfd2), http_api_mux, ip, port);
        } else {
            srs_close_stfd(stfd2);
            srs_warn("Close for invalid fd=%d, ip=%s:%d", fd, ip.c_str(), port);
            return err;
        }
    }

#ifdef SRS_RTC
    // For RTC TCP connection, use resource executor to manage the resource.
    SrsRtcTcpConn* raw_conn = dynamic_cast<SrsRtcTcpConn*>(resource);
    if (raw_conn) {
        SrsSharedResource<SrsRtcTcpConn>* conn = new SrsSharedResource<SrsRtcTcpConn>(raw_conn);
        SrsExecutorCoroutine* executor = new SrsExecutorCoroutine(_srs_rtc_manager, conn, raw_conn, raw_conn);
        raw_conn->setup_owner(conn, executor, executor);
        if ((err = executor->start()) != srs_success) {
            srs_freep(executor);
            return srs_error_wrap(err, "start executor");
        }
        return err;
    }
#endif

    // Use connection manager to manage all the resources.
    srs_assert(resource);
    conn_manager->add(resource);

    // If connection is a resource to start, start a coroutine to handle it.
    ISrsStartable* conn = dynamic_cast<ISrsStartable*>(resource);
    srs_assert(conn);
    if ((err = conn->start()) != srs_success) {
        return srs_error_wrap(err, "start conn coroutine");
    }

    return err;
}