Skip to content

Conversation

@pavelhoral
Copy link
Contributor

@pavelhoral pavelhoral commented Jun 14, 2024

This PR fixes premature start of the glob stream which can lead to closing the resulting Transform stream before it is being properly processed by upstream components. This can lead to very strange race conditions and unfinished pipelines.

Closes #135

@phated
Copy link
Member

phated commented Jun 14, 2024

This is awesome. Thanks for debugging and adding a regression test! I think we can actually leverage the open function in streamx for this.

@pavelhoral
Copy link
Contributor Author

pavelhoral commented Jun 15, 2024

This is awesome. Thanks for debugging and adding a regression test! I think we can actually leverage the open function in streamx for this.

Not sure why, but moving the code to open function breaks the pipeline completely. Seems like open state is too soon for data being produced by the stream.

UPDATE: I am dumb... forgot about the callback.

@pavelhoral pavelhoral force-pushed the fix-premature-start branch from f022a1c to 81552cc Compare June 15, 2024 07:18
@pavelhoral
Copy link
Contributor Author

Pushed new changes (switch to open method).

@pavelhoral
Copy link
Contributor Author

Any update on this? Is there anything else I can do? Ty.

@pavelhoral
Copy link
Contributor Author

Any update on this? Is there anything else I can do? Ty.

Would love to have this merged and released. If there is anything I can do to help that, let me know.

@yocontra yocontra requested review from phated and yocontra December 3, 2024 02:22
Copy link
Member

@yocontra yocontra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me, will let @phated weigh in here

@phated phated changed the title fix: Do not start and end the stream prematurely (#135) fix: Do not start and end the stream prematurely Jun 1, 2025
@phated phated force-pushed the fix-premature-start branch from 950413f to 9011e7a Compare June 1, 2025 04:35
@phated
Copy link
Member

phated commented Jun 1, 2025

@pavelhoral Sorry for such a long delay—life kicked my butt last year. I want to say that your work here is greatly appreciated! You performed amazing debugging and the fix is great.

I wanted to give this PR the attention it deserved, which lead me to finding the regression test didn't fail on main. I figured out that inserting a transform stream resulted in the failure, so I pushed that on the branch. I also rebased to get some CI fixes that I just landed.

@phated phated merged commit 9d36482 into gulpjs:master Jun 1, 2025
24 checks passed
@phated
Copy link
Member

phated commented Jun 1, 2025

This has been released as v8.0.3

Again, sorry for the really long delay and thank you so much for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strange race condition on empy streams

3 participants