Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent apps with the same folder name to colide #270

Merged
merged 1 commit into from
Jan 13, 2021
Merged

Prevent apps with the same folder name to colide #270

merged 1 commit into from
Jan 13, 2021

Conversation

hellvinz
Copy link
Contributor

@hellvinz hellvinz commented Jan 6, 2021

Goal

Before this commit if two apps were resolving their symlinks to distinct folders with the same name, the first app booted was used to serve queries targeting the other app.
example:

ls -la ~/.puma-dev/
lrwxr-xr-x    1 vincent  staff    37  4 jan 15:54 content -> /Users/vincent/Projects/content/rails
lrwxr-xr-x    1 vincent  staff    38 24 déc 11:18 platform -> /Users/vincent/Projects/platform/rails

both content and platform rails app were served by the first of the two apps being booted

With this commit a checksum of the full app path is being added to the app name, which maintain the existing behavior of having a single app booted even if several symlinks are targeting it, but distinct folders with the same name are now treated as different apps

consequences

logs are now displaying lines with the app name suffixed by the folder path checksum:

rails-4506dc3c[29999]: - Gracefully stopping, waiting for requests to finish
rails-deadc98a[29050]: - Gracefully stopping, waiting for requests to finish

before this commit if two symlinks were targeting distinct folders having the same name the app were considered to be the same
@nonrational nonrational self-requested a review January 7, 2021 01:34
@hellvinz
Copy link
Contributor Author

@nonrational anything I can do?

Copy link
Member

@nonrational nonrational left a comment

Choose a reason for hiding this comment

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

LGTM. Will merge and release in the next 24-48hrs.

@nonrational nonrational merged commit 566b636 into puma:master Jan 13, 2021
@nonrational
Copy link
Member

All set. 0.15.2 is in Homebrew. Thank you!

jorgemanrubia added a commit to jorgemanrubia/puma-dev that referenced this pull request Jul 9, 2021
This fixes a problem where it fails to reuse the same instance of the app
when using multiple hostnames with different names (multiple symlinks pointing
to the same folder).

The bug was introduced in puma#270. The patch is based on adding the checksum in
all cases when a symlink is involved, not only when the names don't match. If not,
it can happen that it's not added if the name matches for the first execution, which
results in not finding the app instance later, resulting in an error "There is already
a server bound to..."

Fixes puma#280
nonrational pushed a commit that referenced this pull request Jul 9, 2021
This fixes a problem where it fails to reuse the same instance of the app
when using multiple hostnames with different names (multiple symlinks pointing
to the same folder).

The bug was introduced in #270. The patch is based on adding the checksum in
all cases when a symlink is involved, not only when the names don't match. If not,
it can happen that it's not added if the name matches for the first execution, which
results in not finding the app instance later, resulting in an error "There is already
a server bound to..."

Fixes #280
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.

2 participants