fix watchers never being cleaned up#18559
Closed
Gazler wants to merge 1 commit intotailwindlabs:mainfrom
Closed
Conversation
Previously, there was an issue, where a slow compilation could cause the watchers to build up and never be cleaned. This would result in both excessive logging and the compilation time increasing exponentially. This fix is a naive patch, simply locking if there is a build in progress, and unlocking when it completes.
77f6fca to
b62a2a6
Compare
Contributor
|
Hey @Gazler! Sorry for the super late response to this. Instead of a lock, have you considered adding the change callbacks to a queue and then waiting for the async function to complete before starting the next batch? 🤔 |
Contributor
|
@Gazler Looked into it some more and pushed up a different approach that resolves the issue. Check out #18905. Going to use that instead of this PR now and will see to get this merged ASAP. Thank you so much for the repro!! This was super helpful. But something about ignoring certain updates didn't feel quite right in your proposed solution. Going to add you as co-author though! |
philipp-spiess
added a commit
that referenced
this pull request
Sep 9, 2025
This PR supersets #18559 and fixes the same issue reported by @Gazler. Upon testing, we noticed that it's possible that two parallel invocations of file system change events could cause some cleanup functions to get swallowed. This happens because we only remember one global cleanup function but it is possible timing wise that two calls to `createWatcher()` are created before the old watchers are cleaned and thus only one of the new cleanup functions get retained. To fix this, this PR changes `cleanupWatchers` to an array and ensures that all functions are retained. In some local testing, I was able to trigger this, based on the reproduction by @Gazler in #18559, to often call a cleanup with more than one cleanup function in the array. I'm going to paste the amazing reproduction from #18559 here as well: # Requirements We need a way to stress the CPU to slow down tailwind compilation, for example stress-ng. ``` stress-ng --cpu 16 --timeout 10 ``` It can be install with apt, homebrew or similar. # Installation There is a one-liner at the bottom to perform the required setup and run the tailwindcli. Create a new directory: ```shell mkdir twtest && cd twtest ``` Create a package.json with the correct deps. ```shell cat << 'EOF' > package.json { "dependencies": { "@tailwindcss/cli": "^4.1.11", "daisyui": "^5.0.46", "tailwindcss": "^4.1.11" } } EOF ``` Create the input css: ```shell mkdir src cat << 'EOF' > src/.input.css @import "tailwindcss" source(none); @plugin "daisyui"; @source "../core_components.ex"; @source "../home.html.heex"; @source "./input.css"; EOF ``` Install tailwind, daisyui, and some HTML to make tailwind do some work: ``` npm install wget https://raw.githubusercontent.com/phoenixframework/phoenix/refs/heads/main/installer/templates/phx_web/components/core_components.ex wget https://github.com/phoenixframework/phoenix/blob/main/installer/templates/phx_web/controllers/page_html/home.html.heex ``` # Usage This is easiest with 3 terminal windows: Start a tailwindcli watcher in one terminal: ```shell npx @tailwindcss/cli -i src/input.css -o src/output.css --watch ``` Start a stress test in another: ```shell stress-ng --cpu 16 --timeout 30 ``` Force repeated compilation in another: ```shell for i in $(seq 1 50); do touch src/input.css; sleep 0.1; done ``` # Result Once the stress test has completed, you can run: ```shell touch src/input.css ``` You should see that there is repeated output, and the duration is in the multiple seconds. If this setup doesn't cause the issue, you can also add the `-p` flag which causes the CSS to be printed, slowing things down further: ```shell npx @tailwindcss/cli -i src/input.css -p --watch ``` ## One-liner ```shell mkdir twtest && cd twtest cat << 'EOF' > package.json { "dependencies": { "@tailwindcss/cli": "^4.1.11", "daisyui": "^5.0.46", "tailwindcss": "^4.1.11" } } EOF mkdir src cat << 'EOF' > src/input.css @import "tailwindcss" source(none); @plugin "daisyui"; @source "../core_components.ex"; @source "../home.html.heex"; @source "./input.css"; EOF npm install wget https://raw.githubusercontent.com/phoenixframework/phoenix/refs/heads/main/installer/templates/phx_web/components/core_components.ex wget https://github.com/phoenixframework/phoenix/blob/main/installer/templates/phx_web/controllers/page_html/home.html.heex npx @tailwindcss/cli -i src/input.css -o src/output.css --watch ``` ## Test plan - Not able to reproduce this with a local build of the CLI after the patch is applied but was able to reproduce it again once the patch was reverted. Co-authored-by: Gary Rennie <gazler@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, there was an issue, where a slow compilation could cause the watchers to build up and never be cleaned. This would result in both excessive logging and the compilation time increasing exponentially.
This fix is a naive patch, simply locking if there is a build in progress, and unlocking when it completes. I'm marking it as a draft because I believe this lock can get stuck in a locked state resulting in the project never rebuilding.
The setup for this is a little involved, as it requires slowing down tailwind compilation enough so that the callback is triggered multiple times concurrently, but using the script below I was able to reproduce it fairly reliably:
Requirements
We need a way to stress the CPU to slow down tailwind compilation, for example stress-ng.
It can be install with apt, homebrew or similar.
Installation
There is a one-liner at the bottom to perform the required setup and run the tailwindcli.
Create a new directory:
Create a package.json with the correct deps.
Create the input css:
Install tailwind, daisyui, and some HTML to make tailwind do some work:
Usage
This is easiest with 3 terminal windows:
Start a tailwindcli watcher in one terminal:
Start a stress test in another:
Force repeated compilation in another:
Result
Once the stress test has completed, you can run:
You should see that there is repeated output, and the duration is in the multiple seconds.
If this setup doesn't cause the issue, you can also add the
-pflag which causes theCSS to be printed, slowing things down further:
One-liner