resolves #37: closed connection on large files

The old implementation slurped the whole artefact into memory, and
did not ensure it remained allocated beyond the first call to write().
The new implementation uses mmap and ensures the mapping lasts until
the file has been delivered

src/interface.h

@@ -80,11 +80,18 @@ struct LaminarWaiter {
     virtual void complete(const Run*) = 0;
 };
 
+// Represents a file mapped in memory. Used to serve artefacts
+struct MappedFile {
+    virtual ~MappedFile() =default;
+    virtual const void* address() = 0;
+    virtual size_t size() = 0;
+};
+
 // The interface connecting the network layer to the application business
 // logic. These methods fulfil the requirements of both the HTTP/Websocket
 // and RPC interfaces.
 struct LaminarInterface {
-    virtual ~LaminarInterface() =default;
+    virtual ~LaminarInterface() {}
 
     // Queues a job, returns immediately. Return value will be nullptr if
     // the supplied name is not a known job.
@@ -118,9 +125,9 @@ struct LaminarInterface {
     virtual bool setParam(std::string job, uint buildNum, std::string param, std::string value) = 0;
 
     // Fetches the content of an artifact given its filename relative to
-    // $LAMINAR_HOME/archive. This shouldn't be used, because the sysadmin
+    // $LAMINAR_HOME/archive. Ideally, this would instead be served by a
-    // should have configured a real webserver to serve these things.
+    // proper web server which handles this url.
-    virtual bool getArtefact(std::string path, std::string& result) = 0;
+    virtual kj::Own<MappedFile> getArtefact(std::string path) = 0;
 
     // Fetches the content of $LAMINAR_HOME/custom/style.css or an empty
     // string. This shouldn't be used, because the sysadmin should have

src/laminar.cpp

@@ -22,6 +22,10 @@
 #include "log.h"
 
 #include <sys/wait.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 #include <fstream>
 #include <zlib.h>
 
@@ -840,13 +844,37 @@ static bool slurp(fs::path path, std::string& output) {
     return false;
 }
 
-bool Laminar::getArtefact(std::string path, std::string& result) {
+class MappedFileImpl : public MappedFile {
-    if(archiveUrl != ARCHIVE_URL_DEFAULT) {
+public:
-        // we shouldn't have got here. Probably an invalid link.
+    MappedFileImpl(const char* path) :
-        return false;
+        fd(open(path, O_RDONLY)),
+        sz(0),
+        ptr(nullptr)
+    {
+        if(fd == -1) return;
+        struct stat st;
+        if(fstat(fd, &st) != 0) return;
+        sz = st.st_size;
+        ptr = mmap(nullptr, sz, PROT_READ, MAP_SHARED, fd, 0);
+        if(ptr == MAP_FAILED)
+            ptr = nullptr;
     }
-    fs::path file(fs::path(homeDir)/"archive"/path);
+    ~MappedFileImpl() override {
-    return slurp(file, result);
+        if(ptr)
+            munmap(ptr, sz);
+        if(fd != -1)
+            close(fd);
+    }
+    virtual const void* address() override { return ptr; }
+    virtual size_t size() override { return sz; }
+private:
+    int fd;
+    size_t sz;
+    void* ptr;
+};
+
+kj::Own<MappedFile> Laminar::getArtefact(std::string path) {
+    return kj::heap<MappedFileImpl>(fs::path(fs::path(homeDir)/"archive"/path).c_str());
 }
 
 std::string Laminar::getCustomCss()

src/laminar.h

@@ -55,7 +55,7 @@ public:
 
     void sendStatus(LaminarClient* client) override;
     bool setParam(std::string job, uint buildNum, std::string param, std::string value) override;
-    bool getArtefact(std::string path, std::string& result) override;
+    kj::Own<MappedFile> getArtefact(std::string path) override;
     std::string getCustomCss() override;
     void abortAll() override;
     void reapChildren() override;

src/server.cpp

@@ -311,19 +311,18 @@ private:
             const char* start, *end, *content_type;
             responseHeaders.clear();
             if(resource.compare(0, strlen("/archive/"), "/archive/") == 0) {
-                std::string file(resource.substr(strlen("/archive/")));
+                kj::Own<MappedFile> file = laminar.getArtefact(resource.substr(strlen("/archive/")));
-                std::string content;
+                if(file->address() != nullptr) {
-                if(laminar.getArtefact(file, content)) {
                     responseHeaders.add("Content-Transfer-Encoding", "binary");
-                    auto stream = response.send(200, "OK", responseHeaders, content.size());
+                    auto stream = response.send(200, "OK", responseHeaders, file->size());
-                    return stream->write(content.data(), content.size()).attach(kj::mv(stream));
+                    return stream->write(file->address(), file->size()).attach(kj::mv(file)).attach(kj::mv(stream));
                 }
             } else if(resource.compare("/custom/style.css") == 0) {
                 responseHeaders.set(kj::HttpHeaderId::CONTENT_TYPE, "text/css; charset=utf-8");
                 responseHeaders.add("Content-Transfer-Encoding", "binary");
                 std::string css = laminar.getCustomCss();
                 auto stream = response.send(200, "OK", responseHeaders, css.size());
-                return stream->write(css.data(), css.size()).attach(kj::mv(stream));
+                return stream->write(css.data(), css.size()).attach(kj::mv(css)).attach(kj::mv(stream));
             } else if(resources.handleRequest(resource, &start, &end, &content_type)) {
                 responseHeaders.set(kj::HttpHeaderId::CONTENT_TYPE, content_type);
                 responseHeaders.add("Content-Encoding", "gzip");

test/test-server.cpp

@@ -44,6 +44,7 @@ public:
 class MockLaminar : public LaminarInterface {
 public:
     LaminarClient* client = nullptr;
+    ~MockLaminar() {}
     virtual void registerClient(LaminarClient* c) override {
         ASSERT_EQ(nullptr, client);
         client = c;
@@ -55,12 +56,14 @@ public:
         client = nullptr;
     }
 
+    // MOCK_METHOD does not seem to work with return values whose destructors have noexcept(false)
+    kj::Own<MappedFile> getArtefact(std::string path) override { return kj::Own<MappedFile>(nullptr, kj::NullDisposer()); }
+
     MOCK_METHOD2(queueJob, std::shared_ptr<Run>(std::string name, ParamMap params));
     MOCK_METHOD1(registerWaiter, void(LaminarWaiter* waiter));
     MOCK_METHOD1(deregisterWaiter, void(LaminarWaiter* waiter));
     MOCK_METHOD1(sendStatus, void(LaminarClient* client));
     MOCK_METHOD4(setParam, bool(std::string job, uint buildNum, std::string param, std::string value));
-    MOCK_METHOD2(getArtefact, bool(std::string path, std::string& result));
     MOCK_METHOD0(getCustomCss, std::string());
     MOCK_METHOD0(abortAll, void());
     MOCK_METHOD0(reapChildren, void());