From 5ab96e0d40a61761b0cbfca855b5bc83db743f24 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 7 Jun 2018 18:27:56 +0200 Subject: [PATCH] Wire: Implement MapWriteAsync The flow of commands is a bit more involved than for MapReadAsync and goes like this: - C->S MapAsync isWrite = true - S: Call MapWriteAsync - S: MapWriteAsync callback fired - S->C: MapWriteAsyncCallback (no data compared to the read case) - C: Call the MapWriteAsync callback with a zeroed out buffer - C: Application calls unmap. - C->S: UpdateMappedData with the content of the mapped pointer - S: Copy the data in the mapped pointer - C->S: Regular unmap command - S: Call unmap Makes nxt_end2end_tests -w pass all tests. Also duplicates the MapRead wire tests for the write cases --- generator/templates/wire/WireClient.cpp | 146 ++++++++++++--- generator/templates/wire/WireCmd.h | 4 +- generator/templates/wire/WireServer.cpp | 127 +++++++++++-- src/tests/unittests/WireTests.cpp | 239 ++++++++++++++++++++++-- src/wire/WireCmd.h | 29 ++- 5 files changed, 484 insertions(+), 61 deletions(-) diff --git a/generator/templates/wire/WireClient.cpp b/generator/templates/wire/WireClient.cpp index 103e7755f0..dc55254beb 100644 --- a/generator/templates/wire/WireClient.cpp +++ b/generator/templates/wire/WireClient.cpp @@ -89,25 +89,33 @@ namespace nxt { namespace wire { } void ClearMapRequests(nxtBufferMapAsyncStatus status) { - for (auto& it : readRequests) { - it.second.callback(status, nullptr, it.second.userdata); + for (auto& it : requests) { + if (it.second.isWrite) { + it.second.writeCallback(status, nullptr, it.second.userdata); + } else { + it.second.readCallback(status, nullptr, it.second.userdata); + } } - readRequests.clear(); + requests.clear(); } //* We want to defer all the validation to the server, which means we could have multiple //* map request in flight at a single time and need to track them separately. //* On well-behaved applications, only one request should exist at a single time. - struct MapReadRequestData { - nxtBufferMapReadCallback callback = nullptr; + struct MapRequestData { + nxtBufferMapReadCallback readCallback = nullptr; + nxtBufferMapWriteCallback writeCallback = nullptr; nxtCallbackUserdata userdata = 0; uint32_t size = 0; + bool isWrite = false; }; - std::map readRequests; - uint32_t readRequestSerial = 0; + std::map requests; + uint32_t requestSerial = 0; //* Only one mapped pointer can be active at a time because Unmap clears all the in-flight requests. void* mappedData = nullptr; + size_t mappedDataSize = 0; + bool isWriteMapped = false; }; //* TODO(cwallez@chromium.org): Do something with objects before they are destroyed ? @@ -314,28 +322,47 @@ namespace nxt { namespace wire { {% endfor %} void ClientBufferMapReadAsync(Buffer* buffer, uint32_t start, uint32_t size, nxtBufferMapReadCallback callback, nxtCallbackUserdata userdata) { - uint32_t serial = buffer->readRequestSerial++; - ASSERT(buffer->readRequests.find(serial) == buffer->readRequests.end()); + uint32_t serial = buffer->requestSerial++; + ASSERT(buffer->requests.find(serial) == buffer->requests.end()); - Buffer::MapReadRequestData request; - request.callback = callback; + Buffer::MapRequestData request; + request.readCallback = callback; request.userdata = userdata; request.size = size; - buffer->readRequests[serial] = request; + request.isWrite = false; + buffer->requests[serial] = request; - wire::BufferMapReadAsyncCmd cmd; + wire::BufferMapAsyncCmd cmd; cmd.bufferId = buffer->id; cmd.requestSerial = serial; cmd.start = start; cmd.size = size; + cmd.isWrite = false; auto allocCmd = static_cast(buffer->device->GetCmdSpace(sizeof(cmd))); *allocCmd = cmd; } - void ClientBufferMapWriteAsync(Buffer*, uint32_t, uint32_t, nxtBufferMapWriteCallback, nxtCallbackUserdata) { - // TODO(cwallez@chromium.org): Implement the wire for BufferMapWriteAsync - ASSERT(false); + void ClientBufferMapWriteAsync(Buffer* buffer, uint32_t start, uint32_t size, nxtBufferMapWriteCallback callback, nxtCallbackUserdata userdata) { + uint32_t serial = buffer->requestSerial++; + ASSERT(buffer->requests.find(serial) == buffer->requests.end()); + + Buffer::MapRequestData request; + request.writeCallback = callback; + request.userdata = userdata; + request.size = size; + request.isWrite = true; + buffer->requests[serial] = request; + + wire::BufferMapAsyncCmd cmd; + cmd.bufferId = buffer->id; + cmd.requestSerial = serial; + cmd.start = start; + cmd.size = size; + cmd.isWrite = true; + + auto allocCmd = static_cast(buffer->device->GetCmdSpace(sizeof(cmd))); + *allocCmd = cmd; } void ProxyClientBufferUnmap(nxtBuffer cBuffer) { @@ -349,6 +376,20 @@ namespace nxt { namespace wire { //* - Unmap locally on the client //* - Server -> Client: Result of MapRequest2 if (buffer->mappedData) { + + // If the buffer was mapped for writing, send the update to the data to the server + if (buffer->isWriteMapped) { + wire::BufferUpdateMappedDataCmd cmd; + cmd.bufferId = buffer->id; + cmd.dataLength = static_cast(buffer->mappedDataSize); + + auto allocCmd = static_cast(buffer->device->GetCmdSpace(sizeof(cmd))); + *allocCmd = cmd; + + void* dataAlloc = buffer->device->GetCmdSpace(cmd.dataLength); + memcpy(dataAlloc, buffer->mappedData, cmd.dataLength); + } + free(buffer->mappedData); buffer->mappedData = nullptr; } @@ -369,7 +410,7 @@ namespace nxt { namespace wire { } // Some commands don't have a custom wire format, but need to be handled manually to update - // some client-side state tracking. For these we have to functions: + // some client-side state tracking. For these we have two functions: // - An autogenerated Client{{suffix}} method that sends the command on the wire // - A manual ProxyClient{{suffix}} method that will be inserted in the proctable instead of // the autogenerated one, and that will have to call Client{{suffix}} @@ -412,6 +453,9 @@ namespace nxt { namespace wire { case ReturnWireCmd::BufferMapReadAsyncCallback: success = HandleBufferMapReadAsyncCallback(&commands, &size); break; + case ReturnWireCmd::BufferMapWriteAsyncCallback: + success = HandleBufferMapWriteAsyncCallback(&commands, &size); + break; default: success = false; } @@ -517,18 +561,22 @@ namespace nxt { namespace wire { } //* The requests can have been deleted via an Unmap so this isn't an error. - auto requestIt = buffer->readRequests.find(cmd->requestSerial); - if (requestIt == buffer->readRequests.end()) { + auto requestIt = buffer->requests.find(cmd->requestSerial); + if (requestIt == buffer->requests.end()) { return true; } + //* It is an error for the server to call the read callback when we asked for a map write + if (requestIt->second.isWrite) { + return false; + } + auto request = requestIt->second; - // Delete the request before calling the callback otherwise the callback could be fired a second time if for example buffer.Unmap() is called inside the callback. - buffer->readRequests.erase(requestIt); + //* Delete the request before calling the callback otherwise the callback could be fired a second time. If, for example, buffer.Unmap() is called inside the callback. + buffer->requests.erase(requestIt); //* On success, we copy the data locally because the IPC buffer isn't valid outside of this function if (cmd->status == NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS) { - //* The server didn't send the right amount of data, this is an error and could cause //* the application to crash if we did call the callback. if (request.size != cmd->dataLength) { @@ -544,12 +592,62 @@ namespace nxt { namespace wire { return false; } + buffer->isWriteMapped = false; + buffer->mappedDataSize = request.size; buffer->mappedData = malloc(request.size); memcpy(buffer->mappedData, requestData, request.size); - request.callback(static_cast(cmd->status), buffer->mappedData, request.userdata); + request.readCallback(static_cast(cmd->status), buffer->mappedData, request.userdata); } else { - request.callback(static_cast(cmd->status), nullptr, request.userdata); + request.readCallback(static_cast(cmd->status), nullptr, request.userdata); + } + + return true; + } + + bool HandleBufferMapWriteAsyncCallback(const char** commands, size_t* size) { + const auto* cmd = GetCommand(commands, size); + if (cmd == nullptr) { + return false; + } + + auto* buffer = mDevice->buffer.GetObject(cmd->bufferId); + uint32_t bufferSerial = mDevice->buffer.GetSerial(cmd->bufferId); + + //* The buffer might have been deleted or recreated so this isn't an error. + if (buffer == nullptr || bufferSerial != cmd->bufferSerial) { + return true; + } + + //* The requests can have been deleted via an Unmap so this isn't an error. + auto requestIt = buffer->requests.find(cmd->requestSerial); + if (requestIt == buffer->requests.end()) { + return true; + } + + //* It is an error for the server to call the write callback when we asked for a map read + if (!requestIt->second.isWrite) { + return false; + } + + auto request = requestIt->second; + //* Delete the request before calling the callback otherwise the callback could be fired a second time. If, for example, buffer.Unmap() is called inside the callback. + buffer->requests.erase(requestIt); + + //* On success, we copy the data locally because the IPC buffer isn't valid outside of this function + if (cmd->status == NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS) { + if (buffer->mappedData != nullptr) { + return false; + } + + buffer->isWriteMapped = true; + buffer->mappedDataSize = request.size; + buffer->mappedData = malloc(request.size); + memset(buffer->mappedData, 0, request.size); + + request.writeCallback(static_cast(cmd->status), buffer->mappedData, request.userdata); + } else { + request.writeCallback(static_cast(cmd->status), nullptr, request.userdata); } return true; diff --git a/generator/templates/wire/WireCmd.h b/generator/templates/wire/WireCmd.h index b98617956e..99c25434c9 100644 --- a/generator/templates/wire/WireCmd.h +++ b/generator/templates/wire/WireCmd.h @@ -59,7 +59,8 @@ namespace nxt { namespace wire { {% endfor %} {{as_MethodSuffix(type.name, Name("destroy"))}}, {% endfor %} - BufferMapReadAsync, + BufferMapAsync, + BufferUpdateMappedDataCmd, }; {% for type in by_category["object"] %} @@ -124,6 +125,7 @@ namespace nxt { namespace wire { {{type.name.CamelCase()}}ErrorCallback, {% endfor %} BufferMapReadAsyncCallback, + BufferMapWriteAsyncCallback, }; //* Command for the server calling a builder status callback. diff --git a/generator/templates/wire/WireServer.cpp b/generator/templates/wire/WireServer.cpp index af28007603..70dd018a73 100644 --- a/generator/templates/wire/WireServer.cpp +++ b/generator/templates/wire/WireServer.cpp @@ -27,12 +27,13 @@ namespace nxt { namespace wire { namespace server { class Server; - struct MapReadUserdata { + struct MapUserdata { Server* server; uint32_t bufferId; uint32_t bufferSerial; uint32_t requestSerial; uint32_t size; + bool isWrite; }; //* Stores what the backend knows about the type. @@ -54,6 +55,10 @@ namespace nxt { namespace wire { //* Whether this object has been allocated, used by the KnownObjects queries //* TODO(cwallez@chromium.org): make this an internal bit vector in KnownObjects. bool allocated; + + //* TODO(cwallez@chromium.org): this is only useful for buffers + void* mappedData = nullptr; + size_t mappedDataSize = 0; }; //* Keeps track of the mapping between client IDs and backend objects. @@ -143,6 +148,7 @@ namespace nxt { namespace wire { {% endfor %} void ForwardBufferMapReadAsync(nxtBufferMapAsyncStatus status, const void* ptr, nxtCallbackUserdata userdata); + void ForwardBufferMapWriteAsync(nxtBufferMapAsyncStatus status, void* ptr, nxtCallbackUserdata userdata); // A really really simple implementation of the DeserializeAllocator. It's main feature // is that it has some inline storage so as to avoid allocations for the majority of @@ -253,7 +259,7 @@ namespace nxt { namespace wire { } {% endfor %} - void OnMapReadAsyncCallback(nxtBufferMapAsyncStatus status, const void* ptr, MapReadUserdata* data) { + void OnMapReadAsyncCallback(nxtBufferMapAsyncStatus status, const void* ptr, MapUserdata* data) { ReturnBufferMapReadAsyncCallbackCmd cmd; cmd.bufferId = data->bufferId; cmd.bufferSerial = data->bufferSerial; @@ -274,6 +280,27 @@ namespace nxt { namespace wire { delete data; } + void OnMapWriteAsyncCallback(nxtBufferMapAsyncStatus status, void* ptr, MapUserdata* data) { + ReturnBufferMapWriteAsyncCallbackCmd cmd; + cmd.bufferId = data->bufferId; + cmd.bufferSerial = data->bufferSerial; + cmd.requestSerial = data->requestSerial; + cmd.status = status; + + auto allocCmd = static_cast(GetCmdSpace(sizeof(cmd))); + *allocCmd = cmd; + + if (status == NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS) { + auto* selfData = mKnownBuffer.Get(data->bufferId); + ASSERT(selfData != nullptr); + + selfData->mappedData = ptr; + selfData->mappedDataSize = data->size; + } + + delete data; + } + const char* HandleCommands(const char* commands, size_t size) override { mProcs.deviceTick(mKnownDevice.Get(1)->handle); @@ -294,8 +321,11 @@ namespace nxt { namespace wire { success = Handle{{Suffix}}(&commands, &size); break; {% endfor %} - case WireCmd::BufferMapReadAsync: - success = HandleBufferMapReadAsync(&commands, &size); + case WireCmd::BufferMapAsync: + success = HandleBufferMapAsync(&commands, &size); + break; + case WireCmd::BufferUpdateMappedDataCmd: + success = HandleBufferUpdateMappedData(&commands, &size); break; default: @@ -350,18 +380,35 @@ namespace nxt { namespace wire { //* Helper function for the getting of the command data in command handlers. //* Checks there is enough data left, updates the buffer / size and returns //* the command (or nullptr for an error). - template - static const T* GetCommand(const char** commands, size_t* size) { - if (*size < sizeof(T)) { + template + static const T* GetData(const char** buffer, size_t* size, size_t count) { + // TODO(cwallez@chromium.org): Check for overflow + size_t totalSize = count * sizeof(T); + if (*size < totalSize) { return nullptr; } - const T* cmd = reinterpret_cast(*commands); + const T* data = reinterpret_cast(*buffer); - *commands += sizeof(T); - *size -= sizeof(T); + *buffer += totalSize; + *size -= totalSize; - return cmd; + return data; + } + template + static const T* GetCommand(const char** commands, size_t* size) { + return GetData(commands, size, 1); + } + + {% set custom_pre_handler_commands = ["BufferUnmap"] %} + + bool PreHandleBufferUnmap(const BufferUnmapCmd& cmd) { + auto* selfData = mKnownBuffer.Get(cmd.selfId); + ASSERT(selfData != nullptr); + + selfData->mappedData = nullptr; + + return true; } //* Implementation of the command handlers @@ -379,6 +426,12 @@ namespace nxt { namespace wire { return false; } + {% if Suffix in custom_pre_handler_commands %} + if (!PreHandle{{Suffix}}(cmd)) { + return false; + } + {% endif %} + //* Unpack 'self' auto* selfData = mKnown{{type.name.CamelCase()}}.Get(cmd.selfId); ASSERT(selfData != nullptr); @@ -470,10 +523,10 @@ namespace nxt { namespace wire { } {% endfor %} - bool HandleBufferMapReadAsync(const char** commands, size_t* size) { + bool HandleBufferMapAsync(const char** commands, size_t* size) { //* These requests are just forwarded to the buffer, with userdata containing what the client //* will require in the return command. - const auto* cmd = GetCommand(commands, size); + const auto* cmd = GetCommand(commands, size); if (cmd == nullptr) { return false; } @@ -482,28 +535,63 @@ namespace nxt { namespace wire { uint32_t requestSerial = cmd->requestSerial; uint32_t requestSize = cmd->size; uint32_t requestStart = cmd->start; + bool isWrite = cmd->isWrite; auto* buffer = mKnownBuffer.Get(bufferId); if (buffer == nullptr) { return false; } - auto* data = new MapReadUserdata; + auto* data = new MapUserdata; data->server = this; data->bufferId = bufferId; data->bufferSerial = buffer->serial; data->requestSerial = requestSerial; data->size = requestSize; + data->isWrite = isWrite; auto userdata = static_cast(reinterpret_cast(data)); if (!buffer->valid) { //* Fake the buffer returning a failure, data will be freed in this call. - ForwardBufferMapReadAsync(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); + if (isWrite) { + ForwardBufferMapWriteAsync(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); + } else { + ForwardBufferMapReadAsync(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); + } return true; } - mProcs.bufferMapReadAsync(buffer->handle, requestStart, requestSize, ForwardBufferMapReadAsync, userdata); + if (isWrite) { + mProcs.bufferMapWriteAsync(buffer->handle, requestStart, requestSize, ForwardBufferMapWriteAsync, userdata); + } else { + mProcs.bufferMapReadAsync(buffer->handle, requestStart, requestSize, ForwardBufferMapReadAsync, userdata); + } + + return true; + } + + bool HandleBufferUpdateMappedData(const char** commands, size_t* size) { + const auto* cmd = GetCommand(commands, size); + if (cmd == nullptr) { + return false; + } + + ObjectId bufferId = cmd->bufferId; + size_t dataLength = cmd->dataLength; + + auto* buffer = mKnownBuffer.Get(bufferId); + if (buffer == nullptr || !buffer->valid || buffer->mappedData == nullptr || + buffer->mappedDataSize != dataLength) { + return false; + } + + const char* data = GetData(commands, size, dataLength); + if (data == nullptr) { + return false; + } + + memcpy(buffer->mappedData, data, dataLength); return true; } @@ -524,9 +612,14 @@ namespace nxt { namespace wire { {% endfor %} void ForwardBufferMapReadAsync(nxtBufferMapAsyncStatus status, const void* ptr, nxtCallbackUserdata userdata) { - auto data = reinterpret_cast(static_cast(userdata)); + auto data = reinterpret_cast(static_cast(userdata)); data->server->OnMapReadAsyncCallback(status, ptr, data); } + + void ForwardBufferMapWriteAsync(nxtBufferMapAsyncStatus status, void* ptr, nxtCallbackUserdata userdata) { + auto data = reinterpret_cast(static_cast(userdata)); + data->server->OnMapWriteAsyncCallback(status, ptr, data); + } } CommandHandler* NewServerCommandHandler(nxtDevice device, const nxtProcTable& procs, CommandSerializer* serializer) { diff --git a/src/tests/unittests/WireTests.cpp b/src/tests/unittests/WireTests.cpp index 323bb5b0eb..7c82545ed8 100644 --- a/src/tests/unittests/WireTests.cpp +++ b/src/tests/unittests/WireTests.cpp @@ -97,7 +97,20 @@ class MockBufferMapReadCallback { static MockBufferMapReadCallback* mockBufferMapReadCallback = nullptr; static void ToMockBufferMapReadCallback(nxtBufferMapAsyncStatus status, const void* ptr, nxtCallbackUserdata userdata) { // Assume the data is uint32_t to make writing matchers easier - mockBufferMapReadCallback->Call(status, reinterpret_cast(ptr), userdata); + mockBufferMapReadCallback->Call(status, static_cast(ptr), userdata); +} + +class MockBufferMapWriteCallback { + public: + MOCK_METHOD3(Call, void(nxtBufferMapAsyncStatus status, uint32_t* ptr, nxtCallbackUserdata userdata)); +}; + +static MockBufferMapWriteCallback* mockBufferMapWriteCallback = nullptr; +uint32_t* lastMapWritePointer = nullptr; +static void ToMockBufferMapWriteCallback(nxtBufferMapAsyncStatus status, void* ptr, nxtCallbackUserdata userdata) { + // Assume the data is uint32_t to make writing matchers easier + lastMapWritePointer = static_cast(ptr); + mockBufferMapWriteCallback->Call(status, lastMapWritePointer, userdata); } class WireTestsBase : public Test { @@ -110,6 +123,7 @@ class WireTestsBase : public Test { mockDeviceErrorCallback = new MockDeviceErrorCallback; mockBuilderErrorCallback = new MockBuilderErrorCallback; mockBufferMapReadCallback = new MockBufferMapReadCallback; + mockBufferMapWriteCallback = new MockBufferMapWriteCallback; nxtProcTable mockProcs; nxtDevice mockDevice; @@ -145,6 +159,7 @@ class WireTestsBase : public Test { delete mockDeviceErrorCallback; delete mockBuilderErrorCallback; delete mockBufferMapReadCallback; + delete mockBufferMapWriteCallback; } void FlushClient() { @@ -658,8 +673,10 @@ class WireBufferMappingTests : public WireTestsBase { nxtBuffer errorBuffer; }; -// Check mapping a succesfully created buffer -TEST_F(WireBufferMappingTests, MappingSuccessBuffer) { +// MapRead-specific tests + +// Check mapping for reading a succesfully created buffer +TEST_F(WireBufferMappingTests, MappingForReadSuccessBuffer) { nxtCallbackUserdata userdata = 8653; nxtBufferMapReadAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapReadCallback, userdata); @@ -683,8 +700,8 @@ TEST_F(WireBufferMappingTests, MappingSuccessBuffer) { FlushClient(); } -// Check that things work correctly when a validation error happens when mapping the buffer -TEST_F(WireBufferMappingTests, ErrorWhileMapping) { +// Check that things work correctly when a validation error happens when mapping the buffer for reading +TEST_F(WireBufferMappingTests, ErrorWhileMappingForRead) { nxtCallbackUserdata userdata = 8654; nxtBufferMapReadAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapReadCallback, userdata); @@ -701,8 +718,8 @@ TEST_F(WireBufferMappingTests, ErrorWhileMapping) { FlushServer(); } -// Check mapping a buffer that didn't get created on the server side -TEST_F(WireBufferMappingTests, MappingErrorBuffer) { +// Check mapping for reading a buffer that didn't get created on the server side +TEST_F(WireBufferMappingTests, MappingForReadErrorBuffer) { nxtCallbackUserdata userdata = 8655; nxtBufferMapReadAsync(errorBuffer, 40, sizeof(uint32_t), ToMockBufferMapReadCallback, userdata); @@ -718,8 +735,8 @@ TEST_F(WireBufferMappingTests, MappingErrorBuffer) { FlushClient(); } -// Check that the callback is called with UNKNOWN when the buffer is destroyed before the request is finished -TEST_F(WireBufferMappingTests, DestroyBeforeRequestEnd) { +// Check that the map read callback is called with UNKNOWN when the buffer is destroyed before the request is finished +TEST_F(WireBufferMappingTests, DestroyBeforeReadRequestEnd) { nxtCallbackUserdata userdata = 8656; nxtBufferMapReadAsync(errorBuffer, 40, sizeof(uint32_t), ToMockBufferMapReadCallback, userdata); @@ -729,8 +746,8 @@ TEST_F(WireBufferMappingTests, DestroyBeforeRequestEnd) { nxtBufferRelease(errorBuffer); } -// Check the callback is called with UNKNOWN when the map request would have worked, but Unmap was called -TEST_F(WireBufferMappingTests, UnmapCalledTooEarly) { +// Check the map read callback is called with UNKNOWN when the map request would have worked, but Unmap was called +TEST_F(WireBufferMappingTests, UnmapCalledTooEarlyForRead) { nxtCallbackUserdata userdata = 8657; nxtBufferMapReadAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapReadCallback, userdata); @@ -751,8 +768,8 @@ TEST_F(WireBufferMappingTests, UnmapCalledTooEarly) { FlushServer(); } -// Check that an error callback gets nullptr while a buffer is already mapped -TEST_F(WireBufferMappingTests, MappingErrorWhileAlreadyMappedGetsNullptr) { +// Check that an error map read callback gets nullptr while a buffer is already mapped +TEST_F(WireBufferMappingTests, MappingForReadingErrorWhileAlreadyMappedGetsNullptr) { // Successful map nxtCallbackUserdata userdata = 34098; nxtBufferMapReadAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapReadCallback, userdata); @@ -838,3 +855,199 @@ TEST_F(WireBufferMappingTests, DestroyInsideMapReadCallback) { FlushClient(); } + +// MapWrite-specific tests + +// Check mapping for writing a succesfully created buffer +TEST_F(WireBufferMappingTests, MappingForWriteSuccessBuffer) { + nxtCallbackUserdata userdata = 8653; + nxtBufferMapWriteAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + uint32_t serverBufferContent = 31337; + uint32_t updatedContent = 4242; + uint32_t zero = 0; + + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, 40, sizeof(uint32_t), _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, &serverBufferContent); + })); + + FlushClient(); + + // The map write callback always gets a buffer full of zeroes. + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Pointee(Eq(zero)), userdata)) + .Times(1); + + FlushServer(); + + // Write something to the mapped pointer + *lastMapWritePointer = updatedContent; + + nxtBufferUnmap(buffer); + EXPECT_CALL(api, BufferUnmap(apiBuffer)) + .Times(1); + + FlushClient(); + + // After the buffer is unmapped, the content of the buffer is updated on the server + ASSERT_EQ(serverBufferContent, updatedContent); +} + +// Check that things work correctly when a validation error happens when mapping the buffer for writing +TEST_F(WireBufferMappingTests, ErrorWhileMappingForWrite) { + nxtCallbackUserdata userdata = 8654; + nxtBufferMapWriteAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, 40, sizeof(uint32_t), _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr); + })); + + FlushClient(); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata)) + .Times(1); + + FlushServer(); +} + +// Check mapping for writing a buffer that didn't get created on the server side +TEST_F(WireBufferMappingTests, MappingForWriteErrorBuffer) { + nxtCallbackUserdata userdata = 8655; + nxtBufferMapWriteAsync(errorBuffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + FlushClient(); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata)) + .Times(1); + + FlushServer(); + + nxtBufferUnmap(errorBuffer); + + FlushClient(); +} + +// Check that the map write callback is called with UNKNOWN when the buffer is destroyed before the request is finished +TEST_F(WireBufferMappingTests, DestroyBeforeWriteRequestEnd) { + nxtCallbackUserdata userdata = 8656; + nxtBufferMapWriteAsync(errorBuffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + + nxtBufferRelease(errorBuffer); +} + +// Check the map read callback is called with UNKNOWN when the map request would have worked, but Unmap was called +TEST_F(WireBufferMappingTests, UnmapCalledTooEarlyForWrite) { + nxtCallbackUserdata userdata = 8657; + nxtBufferMapWriteAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + uint32_t bufferContent = 31337; + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, 40, sizeof(uint32_t), _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, &bufferContent); + })); + + FlushClient(); + + // Oh no! We are calling Unmap too early! + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + nxtBufferUnmap(buffer); + + // The callback shouldn't get called, even when the request succeeded on the server side + FlushServer(); +} + +// Check that an error map read callback gets nullptr while a buffer is already mapped +TEST_F(WireBufferMappingTests, MappingForWritingErrorWhileAlreadyMappedGetsNullptr) { + // Successful map + nxtCallbackUserdata userdata = 34098; + nxtBufferMapWriteAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + uint32_t bufferContent = 31337; + uint32_t zero = 0; + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, 40, sizeof(uint32_t), _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, &bufferContent); + })) + .RetiresOnSaturation(); + + FlushClient(); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Pointee(Eq(zero)), userdata)) + .Times(1); + + FlushServer(); + + // Map failure while the buffer is already mapped + userdata ++; + nxtBufferMapWriteAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, 40, sizeof(uint32_t), _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr); + })); + + FlushClient(); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata)) + .Times(1); + + FlushServer(); +} + +// Test that the MapWriteCallback isn't fired twice when unmap() is called inside the callback +TEST_F(WireBufferMappingTests, UnmapInsideMapWriteCallback) { + nxtCallbackUserdata userdata = 2039; + nxtBufferMapWriteAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + uint32_t bufferContent = 31337; + uint32_t zero = 0; + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, 40, sizeof(uint32_t), _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, &bufferContent); + })); + + FlushClient(); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Pointee(Eq(zero)), userdata)) + .WillOnce(InvokeWithoutArgs([&]() { + nxtBufferUnmap(buffer); + })); + + FlushServer(); + + EXPECT_CALL(api, BufferUnmap(apiBuffer)) + .Times(1); + + FlushClient(); +} + +// Test that the MapWriteCallback isn't fired twice the buffer external refcount reaches 0 in the callback +TEST_F(WireBufferMappingTests, DestroyInsideMapWriteCallback) { + nxtCallbackUserdata userdata = 2039; + nxtBufferMapWriteAsync(buffer, 40, sizeof(uint32_t), ToMockBufferMapWriteCallback, userdata); + + uint32_t bufferContent = 31337; + uint32_t zero = 0; + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, 40, sizeof(uint32_t), _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, &bufferContent); + })); + + FlushClient(); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Pointee(Eq(zero)), userdata)) + .WillOnce(InvokeWithoutArgs([&]() { + nxtBufferRelease(buffer); + })); + + FlushServer(); + + EXPECT_CALL(api, BufferRelease(apiBuffer)) + .Times(1); + + FlushClient(); +} diff --git a/src/wire/WireCmd.h b/src/wire/WireCmd.h index 6e1f8d5882..a29c246957 100644 --- a/src/wire/WireCmd.h +++ b/src/wire/WireCmd.h @@ -27,25 +27,42 @@ namespace nxt { namespace wire { size_t messageStrlen; }; - struct BufferMapReadAsyncCmd { - wire::WireCmd commandId = WireCmd::BufferMapReadAsync; + struct BufferMapAsyncCmd { + wire::WireCmd commandId = WireCmd::BufferMapAsync; - uint32_t bufferId; - uint32_t requestSerial; + ObjectId bufferId; + ObjectSerial requestSerial; uint32_t start; uint32_t size; + bool isWrite; }; struct ReturnBufferMapReadAsyncCallbackCmd { wire::ReturnWireCmd commandId = ReturnWireCmd::BufferMapReadAsyncCallback; - uint32_t bufferId; - uint32_t bufferSerial; + ObjectId bufferId; + ObjectSerial bufferSerial; uint32_t requestSerial; uint32_t status; uint32_t dataLength; }; + struct ReturnBufferMapWriteAsyncCallbackCmd { + wire::ReturnWireCmd commandId = ReturnWireCmd::BufferMapWriteAsyncCallback; + + ObjectId bufferId; + ObjectSerial bufferSerial; + uint32_t requestSerial; + uint32_t status; + }; + + struct BufferUpdateMappedDataCmd { + wire::WireCmd commandId = WireCmd::BufferUpdateMappedDataCmd; + + ObjectId bufferId; + uint32_t dataLength; + }; + }} // namespace nxt::wire #endif // WIRE_WIRECMD_H_