diff --git a/bindings/otel-thread-ctx.cc b/bindings/otel-thread-ctx.cc index 69d7aa43..15477a11 100644 --- a/bindings/otel-thread-ctx.cc +++ b/bindings/otel-thread-ctx.cc @@ -614,14 +614,21 @@ void CtxWrap::Append(const FunctionCallbackInfo& args) { isolate->ThrowError("allocation failed"); return; } + // Capture before the copy: the point of the assert below is that the memcpy + // carried the header across intact, not that the record is valid. It used to + // assert `valid == 1`, which invalidate() legitimately makes false — and + // since NDEBUG is not defined for this addon, that aborted release builds + // too, not just debug ones. + const uint8_t src_valid = self->record_->valid; // Copy the existing record (header + already-written attrs_data). memcpy( new_rec.get(), self->record_, sizeof(OtelThreadCtxRecord) + current_used); // Append the new entries and update attrs_data_size. memcpy(&new_rec->attrs_data[current_used], appended.data(), appended.size()); new_rec->attrs_data_size = static_cast(new_used); - // The copy should've preserved valid=1 from the source record. - assert(new_rec->valid == 1); + // The copy should've carried the source record's header across verbatim, + // whatever its validity was. + assert(new_rec->valid == src_valid); // Publish: the pointer swap is the atomic boundary the reader sees. The // first fence keeps the new_rec content writes ordered before the pointer diff --git a/ts/test/otel-invalidate-append.ts b/ts/test/otel-invalidate-append.ts new file mode 100644 index 00000000..9d12dc16 --- /dev/null +++ b/ts/test/otel-invalidate-append.ts @@ -0,0 +1,57 @@ +/* + * Copyright 2026 Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +// Runs in a forked process because the failure mode is an abort, which would +// take the whole mocha run down with it. +// +// Append's reallocate path used to assert that the copied record had +// `valid == 1`. invalidate() sets that byte to 0 and appending afterwards is +// supported, so an append too large to fit in place aborted the process: +// +// Assertion `new_rec->valid == 1' failed. +// +// Only the reallocate path is affected — an append that fits the current +// capacity is written in place and never copies the header. A fresh record has +// 36 bytes of attrs_data capacity (64 - sizeof(header)), so the value below is +// comfortably past it. + +import assert from 'assert'; + +import {otelThreadCtx} from '../src/index'; + +const VALUE = 'x'.repeat(200); + +const ctx = new otelThreadCtx.ThreadContext( + Buffer.alloc(16, 1), + Buffer.alloc(8, 2), +); + +ctx.run(() => { + ctx.invalidate(); + ctx.appendAttributes([VALUE]); + + const bytes = ctx.debugBytes(); + const attrsDataSize = bytes[26] | (bytes[27] << 8); + + // invalidate() must stick: growing the record does not resurrect it. + assert.strictEqual(bytes[24], 0, 'valid byte should still be 0'); + // key index (1) + length (1) + the value itself. + assert.strictEqual(attrsDataSize, VALUE.length + 2, 'attrs_data_size'); +}); + +console.log('ok'); diff --git a/ts/test/test-otel-thread-ctx.ts b/ts/test/test-otel-thread-ctx.ts index f26af9db..f4d6683b 100644 --- a/ts/test/test-otel-thread-ctx.ts +++ b/ts/test/test-otel-thread-ctx.ts @@ -770,6 +770,45 @@ function captureBytes(opts: { }); }); + describe('invalidate then grow', () => { + // Regression test: Append's reallocate path asserted that the copied + // record had valid == 1, which invalidate() legitimately makes false, so + // an append too large to fit in place aborted the process. NDEBUG is not + // defined for this addon, so that hit release builds too. The sibling + // test above appends 6 bytes, which fits the initial 36-byte capacity and + // is written in place, so it never reached the copy. Forked, because the + // failure is an abort rather than a test failure. + it('should survive an append that reallocates after invalidate', async function () { + this.timeout(30000); + + const proc = fork(join(__dirname, 'otel-invalidate-append.js'), { + silent: true, + }); + let output = ''; + proc.stdout?.on('data', chunk => { + output += chunk; + }); + proc.stderr?.on('data', chunk => { + output += chunk; + }); + + await new Promise((resolve, reject) => { + proc.on('error', reject); + proc.on('close', (code, signal) => { + if (code === 0) { + resolve(); + } else { + reject( + new Error( + `otel-invalidate-append exited with code=${code} signal=${signal}\n${output}`, + ), + ); + } + }); + }); + }); + }); + describe('getProcessContextAttributes', () => { it('rejects non-array keys', () => { strictAssert.throws(