Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/jsc/bindings/RejectedPromiseQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ NEVER_INLINE void RejectedPromiseQueue::makeRoom()
m_entries.reserveCapacity(std::max<size_t>(16, capacity * 2));
}

NEVER_INLINE void RejectedPromiseQueue::makeRoomBeforeRead(JSC::JSCell* owner)
{
WTF::Locker locker { owner->cellLock() };
makeRoom();
}

void RejectedPromiseQueue::markReported(JSC::JSGlobalObject* owner, JSC::JSPromise* promise)
{
JSC::VM& vm = owner->vm();
Expand Down
20 changes: 19 additions & 1 deletion src/jsc/bindings/RejectedPromiseQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ class RejectedPromiseQueue {
void markReported(JSC::JSGlobalObject* owner, JSC::JSPromise*);
template<typename Visitor> void visit(JSC::JSCell* owner, Visitor&);
bool isEmpty() const { return m_entries.isEmpty(); }
// The size, after room is made for one append: an append to a full queue can drop handled entries.
size_t sizeBeforeRead(JSC::JSCell* owner);
// Entries enter at the end and never move up: if the queue is longer than `sizeBefore`, its newest entry entered since. Marks that promise as handled if it is `returned`.
void handleRejectionOfRead(size_t sizeBefore, JSC::JSValue returned);

private:
struct Entry {
Expand All @@ -34,8 +38,9 @@ class RejectedPromiseQueue {
static bool isHandled(const Entry& entry) { return static_cast<JSC::JSPromise*>(entry.promise.get().asCell())->isHandled(); }
bool wasReported(JSC::JSPromise*);
void makeRoom();
void makeRoomBeforeRead(JSC::JSCell* owner);

// In rejection order. A handled promise stays until it is at the end, makeRoom() runs, or the drain.
// In rejection order. A handled promise stays until makeRoom() runs or the drain. One that got its handler through didHandle() also leaves at the end.
WTF::Vector<Entry> m_entries;
// Promises reported as unhandled. Weak: a reported promise that dies leaves it.
JSC::WriteBarrier<JSC::JSWeakSet> m_reported;
Expand Down Expand Up @@ -64,4 +69,17 @@ ALWAYS_INLINE bool RejectedPromiseQueue::didHandle(JSC::JSCell* owner, JSC::JSPr
return m_reported && wasReported(promise);
}

ALWAYS_INLINE size_t RejectedPromiseQueue::sizeBeforeRead(JSC::JSCell* owner)
{
if (m_entries.size() == m_entries.capacity() && !m_entries.isEmpty()) [[unlikely]]
makeRoomBeforeRead(owner);
return m_entries.size();
}

ALWAYS_INLINE void RejectedPromiseQueue::handleRejectionOfRead(size_t sizeBefore, JSC::JSValue returned)
{
if (m_entries.size() > sizeBefore && m_entries.last().promise.get() == returned) [[unlikely]]
static_cast<JSC::JSPromise*>(returned.asCell())->markAsHandled();
}

} // namespace Bun
2 changes: 2 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ class GlobalObject : public Bun::GlobalScope {
uint8_t drainMicrotasks();

void handleRejectedPromises();
size_t queuedRejectionCountBeforeRead() { return m_aboutToBeNotifiedRejectedPromises.sizeBeforeRead(this); }
void handleRejectionOfRead(size_t countBefore, JSC::JSValue returned) { m_aboutToBeNotifiedRejectedPromises.handleRejectionOfRead(countBefore, returned); }
ALWAYS_INLINE void* bunVM() const { return m_bunVM; }
#if OS(WINDOWS)
uv_loop_t* uvLoop() const
Expand Down
18 changes: 16 additions & 2 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5316,6 +5316,16 @@ extern "C" void JSGlobalObject__throwStackOverflow(JSC::JSGlobalObject* globalOb
throwStackOverflowError(globalObject, scope);
}

// Only the walk asked for this read, so a promise that the getter rejects in this call and returns is the walk's to handle.
static JSC::JSValue getNativeGetterValueForDisplay(JSC::JSGlobalObject* globalObject, const JSC::PropertySlot& slot, JSC::PropertyName property)
{
auto* rejections = defaultGlobalObject(globalObject);
size_t queuedBefore = rejections->queuedRejectionCountBeforeRead();
JSC::JSValue value = slot.getValue(globalObject, property);
rejections->handleRejectionOfRead(queuedBefore, value);
return value;
}

template<bool nonIndexedOnly>
static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* globalObject, void* arg2, void (*iter)(JSC::JSGlobalObject* arg0, void* ctx, EncodedSlice* arg2, JSC::EncodedJSValue JSValue3, bool isSymbol, bool isPrivateSymbol))
{
Expand Down Expand Up @@ -5517,7 +5527,7 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
} else if (slot.attributes() & PropertyAttribute::BuiltinOrFunction) {
propertyValue = slot.getValue(globalObject, property);
} else if (slot.isCustom()) {
propertyValue = slot.getValue(globalObject, property);
propertyValue = getNativeGetterValueForDisplay(globalObject, slot, property);
} else if (slot.isValue()) {
propertyValue = slot.getValue(globalObject, property);
} else if (object->getOwnPropertySlot(object, globalObject, property, slot)) {
Expand All @@ -5531,6 +5541,8 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
} else {
propertyValue = slot.getPureResult();
}
} else if (slot.isCustom()) {
propertyValue = getNativeGetterValueForDisplay(globalObject, slot, property);
} else {
propertyValue = slot.getValue(globalObject, property);
}
Expand Down Expand Up @@ -5667,7 +5679,7 @@ extern "C" [[ZIG_EXPORT(nothrow)]] bool JSC__isBigIntInInt64Range(JSC::EncodedJS
} else if (slot.attributes() & PropertyAttribute::BuiltinOrFunction) {
propertyValue = slot.getValue(globalObject, property);
} else if (slot.isCustom()) {
propertyValue = slot.getValue(globalObject, property);
propertyValue = getNativeGetterValueForDisplay(globalObject, slot, property);
} else if (slot.isValue()) {
propertyValue = slot.getValue(globalObject, property);
} else if (object->getOwnPropertySlot(object, globalObject, property, slot)) {
Expand All @@ -5676,6 +5688,8 @@ extern "C" [[ZIG_EXPORT(nothrow)]] bool JSC__isBigIntInInt64Range(JSC::EncodedJS
}
} else if ((slot.attributes() & PropertyAttribute::Accessor) != 0) {
propertyValue = slot.getPureResult();
} else if (slot.isCustom()) {
propertyValue = getNativeGetterValueForDisplay(globalObject, slot, property);
} else {
propertyValue = slot.getValue(globalObject, property);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "JSDOMExceptionHandling.h"
#include "JSDOMGlobalObject.h"
#include "JSDOMGlobalObjectInlines.h"
#include "JSDOMPromiseDeferred.h"
#include "JSDOMWrapperCache.h"
#include "JSReadRequest.h"
#include "JSReadableByteStreamController.h"
Expand Down Expand Up @@ -388,7 +389,7 @@ JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamBYOBReaderPrototypeGetter_closed, (JSGl
{
const auto* reader = dynamicDowncast<JSReadableStreamBYOBReader>(JSValue::decode(thisValue));
if (!reader) [[unlikely]]
return JSValue::encode(promiseRejectedWith(lexicalGlobalObject, createTypeError(lexicalGlobalObject, "The 'closed' getter can only be used on a ReadableStreamBYOBReader"_s)));
return createRejectedPromiseWithTypeError(*lexicalGlobalObject, "The 'closed' getter can only be used on a ReadableStreamBYOBReader"_s, RejectedPromiseWithTypeErrorCause::NativeGetter);
return JSValue::encode(reader->m_closedPromise.get());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "JSDOMExceptionHandling.h"
#include "JSDOMGlobalObject.h"
#include "JSDOMGlobalObjectInlines.h"
#include "JSDOMPromiseDeferred.h"
#include "JSDOMWrapperCache.h"
#include "JSDirectStreamController.h"
#include "JSReadRequest.h"
Expand Down Expand Up @@ -692,7 +693,7 @@ JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamDefaultReaderPrototypeGetter_closed, (J
{
const auto* reader = dynamicDowncast<JSReadableStreamDefaultReader>(JSValue::decode(thisValue));
if (!reader) [[unlikely]]
return JSValue::encode(promiseRejectedWith(lexicalGlobalObject, createTypeError(lexicalGlobalObject, "The 'closed' getter can only be used on a ReadableStreamDefaultReader"_s)));
return createRejectedPromiseWithTypeError(*lexicalGlobalObject, "The 'closed' getter can only be used on a ReadableStreamDefaultReader"_s, RejectedPromiseWithTypeErrorCause::NativeGetter);
return JSValue::encode(reader->m_closedPromise.get());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "JSDOMExceptionHandling.h"
#include "JSDOMGlobalObject.h"
#include "JSDOMGlobalObjectInlines.h"
#include "JSDOMPromiseDeferred.h"
#include "JSDOMWrapperCache.h"
#include "JSStreamPipeToOperation.h"
#include "JSWritableStream.h"
Expand Down Expand Up @@ -386,7 +387,7 @@ JSC_DEFINE_CUSTOM_GETTER(jsWritableStreamDefaultWriterPrototypeGetter_closed, (J
{
const auto* writer = dynamicDowncast<JSWritableStreamDefaultWriter>(JSValue::decode(thisValue));
if (!writer) [[unlikely]]
return JSValue::encode(promiseRejectedWith(lexicalGlobalObject, createTypeError(lexicalGlobalObject, "The 'closed' getter can only be used on a WritableStreamDefaultWriter"_s)));
return createRejectedPromiseWithTypeError(*lexicalGlobalObject, "The 'closed' getter can only be used on a WritableStreamDefaultWriter"_s, RejectedPromiseWithTypeErrorCause::NativeGetter);
return JSValue::encode(writer->m_closedPromise.get());
}

Expand All @@ -409,7 +410,7 @@ JSC_DEFINE_CUSTOM_GETTER(jsWritableStreamDefaultWriterPrototypeGetter_ready, (JS
{
auto* writer = dynamicDowncast<JSWritableStreamDefaultWriter>(JSValue::decode(thisValue));
if (!writer) [[unlikely]]
return JSValue::encode(promiseRejectedWith(lexicalGlobalObject, createTypeError(lexicalGlobalObject, "The 'ready' getter can only be used on a WritableStreamDefaultWriter"_s)));
return createRejectedPromiseWithTypeError(*lexicalGlobalObject, "The 'ready' getter can only be used on a WritableStreamDefaultWriter"_s, RejectedPromiseWithTypeErrorCause::NativeGetter);
return JSValue::encode(writer->readyPromise(lexicalGlobalObject));
}

Expand Down
103 changes: 88 additions & 15 deletions test/cli/inspect/inspect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -604,14 +604,10 @@ test("error.stack doesnt lose frames", () => {
expect(no.split("\n").slice(0, -2).join("\n").trim()).toBe(yes.split("\n").slice(0, -2).join("\n").trim());
});

test("LifecycleReporter.getModuleGraph reports an empty argv when process.argv isn't an array", async () => {
await using child = spawn({
cmd: [bunExe(), "--inspect=ws://127.0.0.1:0/", "-e", "setInterval(() => {}, 1000);"],
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
});
type InspectorReply = { result?: any; error?: { message: string } };

// Connects a WebSocket client to a child that runs with `--inspect=ws://127.0.0.1:0/` and a piped stderr.
async function connectToInspector(child: Subprocess<"ignore", "ignore", "pipe">) {
let stderr = "";
let url: string | undefined;
const decoder = new TextDecoder();
Expand All @@ -622,31 +618,43 @@ test("LifecycleReporter.getModuleGraph reports an empty argv when process.argv i
}
if (!url) throw new Error(`Unable to find the inspector URL in:\n${stderr}`);

type Reply = { result?: any; error?: { message: string } };
const pending = new Map<number, (reply: Reply) => void>();
// A crash closes the socket. Resolve instead of rejecting so the failure shows up in the snapshot below.
const closed = Promise.withResolvers<Reply>();
const opened = Promise.withResolvers<Reply>();
const pending = new Map<number, (reply: InspectorReply) => void>();
// A crash closes the socket. Resolve instead of rejecting so the failure shows up in the caller's assertion.
const closed = Promise.withResolvers<InspectorReply>();
const opened = Promise.withResolvers<InspectorReply>();
const webSocket = new WebSocket(url);
webSocket.addEventListener("open", () => opened.resolve({}));
webSocket.addEventListener("close", () => closed.resolve({ error: { message: "the inspector closed the socket" } }));
webSocket.addEventListener("error", () => closed.resolve({ error: { message: "the inspector socket failed" } }));
const events: { method: string; params: any }[] = [];
webSocket.addEventListener("message", ({ data }) => {
const reply: Reply & { id: number } = JSON.parse(data.toString());
const reply: InspectorReply & { id?: number; method: string; params: any } = JSON.parse(data.toString());
if (reply.id === undefined) return void events.push(reply);
pending.get(reply.id)?.(reply);
pending.delete(reply.id);
});

let nextId = 1;
function send(method: string, params: object = {}): Promise<Reply> {
function send(method: string, params: object = {}): Promise<InspectorReply> {
const id = nextId++;
const { promise, resolve } = Promise.withResolvers<Reply>();
const { promise, resolve } = Promise.withResolvers<InspectorReply>();
pending.set(id, resolve);
webSocket.send(JSON.stringify({ id, method, params }));
return Promise.race([promise, closed.promise]);
}

expect(await Promise.race([opened.promise, closed.promise])).toEqual({});
return { send, events, webSocket };
}

test("LifecycleReporter.getModuleGraph reports an empty argv when process.argv isn't an array", async () => {
await using child = spawn({
cmd: [bunExe(), "--inspect=ws://127.0.0.1:0/", "-e", "setInterval(() => {}, 1000);"],
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
});
const { send, webSocket } = await connectToInspector(child);

const argv: Record<string, unknown> = {};
for (const expression of ["undefined", "0", `"abc"`, "{}", `["a", "b"]`]) {
Expand All @@ -671,3 +679,68 @@ test("LifecycleReporter.getModuleGraph reports an empty argv when process.argv i
expect(child.exitCode).toBe(null);
webSocket.close();
});

// A preview reads native getters with the previewed object as the receiver. The `closed` and `ready` getters of
// stream readers and writers return a rejected promise when that receiver is not an instance.
test("a preview of an object that inherits from a stream reader or writer prototype reports no unhandled rejection", async () => {
await using child = spawn({
cmd: [
bunExe(),
"--inspect=ws://127.0.0.1:0/",
"-e",
`globalThis.rejections = [];
process.on("unhandledRejection", error => rejections.push(error.message));
globalThis.reader = Object.create(ReadableStreamDefaultReader.prototype);
globalThis.byobReader = Object.create(ReadableStreamBYOBReader.prototype);
globalThis.writer = Object.create(WritableStreamDefaultWriter.prototype);
globalThis.holder = { reader, byobReader, writer };
setInterval(() => {}, 1000);`,
],
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
});
const { send, events, webSocket } = await connectToInspector(child);
const takeRejections = async () => {
const reply = await send("Runtime.evaluate", { expression: "rejections.splice(0)", returnByValue: true });
return reply.error?.message ?? reply.result.result.value;
};
// The getters that a preview read: it shows their value as a Promise.
const promisesIn = (preview: any) => preview?.properties.filter(p => p.value === "Promise").map(p => p.name);

const getterReads: Record<string, unknown> = {};
for (const expression of ["reader", "byobReader", "writer", "holder"]) {
const evaluated = await send("Runtime.evaluate", { expression, generatePreview: true });
const objectId = evaluated.result?.result.objectId;
const displayable = await send("Runtime.getDisplayableProperties", { objectId, generatePreview: true });
const own = await send("Runtime.getProperties", { objectId, ownProperties: true, generatePreview: true });
getterReads[expression] = {
evaluate: evaluated.error?.message ?? promisesIn(evaluated.result.result.preview),
getDisplayableProperties:
displayable.error?.message ?? displayable.result.properties.filter(p => p.nativeGetter).map(p => p.name),
getProperties: own.error?.message ?? own.result.properties.flatMap(p => promisesIn(p.value?.preview) ?? []),
};
}
// A request that reads no getter tests nothing. This table fails when the inspector stops these reads.
expect(getterReads).toEqual({
reader: { evaluate: ["closed"], getDisplayableProperties: ["closed"], getProperties: [] },
byobReader: { evaluate: ["closed"], getDisplayableProperties: ["closed"], getProperties: [] },
writer: { evaluate: ["closed", "ready"], getDisplayableProperties: ["closed", "ready"], getProperties: [] },
holder: { evaluate: [], getDisplayableProperties: [], getProperties: ["closed", "closed", "closed", "ready"] },
});
expect(await takeRejections()).toEqual([]);

// With the Console domain on, the inspector previews each argument of console.log and the formatter prints it.
await send("Console.enable");
await send("Runtime.evaluate", { expression: "console.log(reader, byobReader, writer)" });
const logged = events.findLast(event => event.method === "Console.messageAdded");
expect(logged?.params.message.parameters.map(parameter => promisesIn(parameter.preview))).toEqual([
["closed"],
["closed"],
["closed", "ready"],
]);
expect(await takeRejections()).toEqual([]);

expect(child.exitCode).toBe(null);
webSocket.close();
});
Loading
Loading