vibe-d / vibe-http

Future vibe.d HTTP implementation
15 stars 9 forks source link

Add CI checks #3

Closed s-ludwig closed 6 years ago

GallaFrancesco commented 6 years ago

The reason for the builds failing is this code in internals/http1.d:

void handleHTTP1RequestChain(ConnectionStream)(ConnectionStream connection, HTTPContext context)
@safe
{
    while(true) {
        // NOTE: this requires that connection has no scoped destruction
        // Currently vibe-d/vibe-core mantains the destructor,
        // which prevents the closure from being built
        auto st = connection.waitForDataAsync( (bool st) @safe {
                if (!st) connection.close;
                else runTask(&handleHTTP1RequestChain, connection, context);
            });

To pass the connection structure to a closure it should not have a destructor. Since I was afraid that removing a destructor from vibe-core/net.d module TCPConnection would cause breakage of the existing code, I resorted to adding a workaround, which I didn't push yet because I wanted to discuss it with you:

void handleHTTP1RequestChain(ConnectionStream)(ConnectionStream connection, HTTPContext context)
@safe
{
    while(true) {
        // workaround
        struct exp {
            ConnectionStream conn;
        }       
        exp e = { conn:connection };
        auto st = connection.waitForDataAsync( (bool st) @safe {
                if (!st) e.conn.close;
                else runTask(&handleHTTP1RequestChain, e.conn, context);
            });

At the moment this is the only way I found to prevent the compiler from failing because of the scoped destruction.

s-ludwig commented 6 years ago

I have committed a proper fix now, along with a fix in vibe-core: https://github.com/vibe-d/vibe-core/pull/89 This still needs to be merged, with a new version tag being added before this will pass.

GallaFrancesco commented 6 years ago

Thank you @s-ludwig, the rest of the code should be working and coherent with the unittest.