prettier / prettier-synchronized

Synchronous version of Prettier
MIT License
23 stars 7 forks source link

Locks in VSCode debug mode on import #22

Open TonyGravagno opened 2 months ago

TonyGravagno commented 2 months ago

v0.5.2 - When a Typescript (ESM Node) project is run in VSCode in debug mode, the initalization process gets to the import of this package and stops:

import synchronizedPrettier from "@prettier/sync";

The return stack shows the Atomic.wait seems to be locked, and when locked on itself (?) there is some conflict with debug mode.
There is no issue when not running in debug mode.

const status = Atomics.wait(semaphore, 0, 0, timeout) ref

The first issue is that the semaphore can't be locked. But there is a second issue, that when this happens there is no timeout, so the process does not terminate.

return this.#sendActionToWorker(this.#worker, action, payload) ref

That line does not include a 'timeout' parameter which is defined on that function:

#sendActionToWorker(worker, action, payload, timeout) { ref

This results in 'timeout' being undefined, resulting in an unterminating lock.

While I'd normally request/recommend a default timeout setting with an option override, that won't help in this case of the import statement locking.

Please consider both of these issues in the resolution(s) of this challenge.

I know this timeout situation is a topic for make-synchronized, but because the author is the same I'm reporting it here. I will create a new issue there on request. Because I'm not familiar with these projects I don't know if I'm not using them correctly and either or both of these issues may not be "bugs" - so I don't want to create another ticket if there is no real problem.

Thanks.

fisker commented 2 months ago

The first issue is that the semaphore can't be locked.

I can't understand what this mean.

But there is a second issue, that when this happens there is no timeout, so the process does not terminate.

Expected, when worker job is done, it will notify the main thread.

TonyGravagno commented 2 months ago

Yes, my original text was incorrect. I am rephrasing the issue:

This is my code:

import * as prettier from "prettier";
import makeSynchronized from "make-synchronized";
export default makeSynchronized(this, async function formatByPrettier(str: string) {
    return await prettier.format(str, {
        parser: "babel-ts",
    });
});

It is called like this:

const formatted = await formatByPrettier(rawCode);

( That is not the exact code, which is nested inside other code. If necessary I will create an isolated case and attach it here. )

When that is run in VSCode from a script that executes tsx, not in debug mode, it runs correctly. The code is beautified by Prettier, and control returns to the calling code above.

When that is run in VSCode in debug mode, the semaphore locks and does not release. VSCode debug shows the Atomics.wait call at the top of the stack.
Maybe the worker process is running on another thread and is not finishing.
Maybe the worker process ( prettier ) is CJS and we cannot one-step through it when debugging ESM, but the UI doesn't show that.

I don't know what's happening. I'm just telling you what happens when I execute your prettier-synchronized code which calls to your make-synchronized code.

I hope that is helpful.

TonyGravagno commented 2 months ago

when this happens there is no timeout, so the process does not terminate.

Expected, when worker job is done, it will notify the main thread.

It is your worker process @prettier/sync that is not finishing.
The process hangs on import synchronizedPrettier from "@prettier/sync";. We (users of your code) can't control that, and we cannot add a timeout there.

The reason for the timeout option in your make-synchronized code is to allow a process to escape if the worker does not exit. In your prettier-synchronized code, you made a choice not to use that. So your code hangs your code.

I am suggesting that you add a timeout and maybe throw an exception if the worker does not complete: return this.#sendActionToWorker(this.#worker, action, payload) // << here

That will not fix the problem with the code hanging in the debugger, but it will allow the code to gracefully exit whenever that situation exists.

Thank you.

fisker commented 2 months ago

I'm not sure what happened either, the timeout in make-synchronized was designed to catch worker initialize error. I don't think it's a good idea to add timeout to the actual job, since there is nothing wrong with running a program costs 1 hour.

Currently the "timeout" only works in development (make-synchronized's unbundled code, nothing related to VSCode's debug mode).

My guess is something wrong when initializing the worker, unfortunately, we can't add "timeout" for worker initialization either, see https://github.com/fisker/make-synchronized/issues/17 and https://github.com/nodejs/node/issues/51679

TonyGravagno commented 2 months ago

I understand, and I thank you for your time on this.

maxpatiiuk commented 1 month ago

I am able to reproduce this issue consistently in a regular Chrome Debugger (not VS Code).

In my case, prettier-synchronized is imported via a dependency of mind, locking down my debugger.