Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about performing blocking tasks in request handlers #257

Closed
cleverdripper opened this issue Jul 6, 2013 · 5 comments
Closed

Question about performing blocking tasks in request handlers #257

cleverdripper opened this issue Jul 6, 2013 · 5 comments

Comments

@cleverdripper
Copy link

In an HTTP request handler, is it ok to make calls to synchronous IO facilities provided by the system? I know vibe does async IO, but I don't know whether it somehow uses libevent to hook all the calls to blocking system IO functions to preempt the tasks or something. I am inexperienced in this field so I don't know what's possible.

The reason I ask is that I need to spawn a process and wait for the output of that process (on a pipe) in creating the response for the HTTP request. I tried making a new thread using std.concurrency.spawn and calling vibe's yield between calls to receiveTimeout with a timeout of 1 millisecond, but I get segmentation faults. Even if it had worked, that's a really suboptimal way to do it, I'd rather do it by notifications than polling. What is the best way to do something like this?


I think the solution is to just use named pipes and treat them as files. Will reopen if this isn't adequate.

EDIT: Nope, tried that too, threadedfile doesn't like reading named pipes, I get the same error as in #256 . Reopening.

@s-ludwig
Copy link
Member

s-ludwig commented Jul 7, 2013

Blocking calls will generally make the event loop dysfunctional, so those need to be replaced or put into a separate thread. Because asynchronous file I/O is currently not implemented for the libevent driver, I have written a thread based wrapper for std.stdio.File that provides an asynchronous Stream interface. I'm also using it for reading the output of a spawned process. I can post it here, but it's not yet in a state in which it should be added to vibe.d (which will probably happen in the form of improving ThreadedFileStream.

@s-ludwig
Copy link
Member

s-ludwig commented Jul 7, 2013

@cleverdripper
Copy link
Author

@s-ludwig Wow, thanks so much. I had tried something similar but I'm new to D so I got bogged down in unfamiliarity.

One thing I ran into is that I can't see a way to stop fileserver from sending the after the callback. For example, if I want to preprocess files with PHP using your excellent StdFileStream class, I can do that and send the output of the PHP interpreter, but then fileserver will try to send the original contents of the file and error out because the body has already been sent. So it seems that all the preWriteCallback is good for is changing headers.

Changing the return type of the pre-write callback would break existing code, so maybe the callback can throw a special exception or something to stop the fileserver from sending the contents of the file. That seems hackish though and uses exceptions for non-exceptional situations. Alternatively, the current preWriteCallback could be marked as deprecated and another callback could be added that had the bool return type, and both callbacks would be called until the deprecated one was removed in the far future.

I don't know how complicated and general-purpose you want fileserver to get, so if I should just quit using it and write my own file serving routes then just say so and I will quit being a pest :)

@s-ludwig
Copy link
Member

s-ludwig commented Jul 8, 2013

I would use the URLRouter to process .php files first and only fall through to the file server for the rest:

router.get("*", &handlePHP);
router.get("*", serveStaticFiles(...));

void handlePHP(HTTPServerRequest req, HTTPServerResponse res)
{
    if (!req.path.endsWith(".php")) return;

    // this block is unfortunately more or less duplicated from the file server
    auto p = Path(req.path);
    p.normalize();
    p.absolute = false;
    if (p.external) return;
    if (!existsFile(p)) return;

    // auto pipes = spawnProcess ...
    scope stdin = new StdFileStream(false, true);
    stdin.setup(pipes.stdin);
    // ...
    res.bodyWriter.write(stdin);
}

The file server could be extended here, but I want to keep it as simple as possible and not introduce features that can be achieved (equally well) in other ways. Using the preWriteCallback also would mean that all the cache, etag and content type headers are set to (probably) wrong values, as the PHP output probably shouldn't be cached, so those need to be cleared as well.

@cleverdripper
Copy link
Author

Alright thanks again, and again great work on vibe, it's such an amazing library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants