Skip to content

Include pid in names of temporary files - #290

Merged
ronwsmith merged 1 commit into
oesmith:masterfrom
clayton-shopify:fix-race-condition
Aug 27, 2020
Merged

Include pid in names of temporary files#290
ronwsmith merged 1 commit into
oesmith:masterfrom
clayton-shopify:fix-race-condition

Conversation

@clayton-shopify

@clayton-shopify clayton-shopify commented Jun 12, 2020

Copy link
Copy Markdown
Contributor

When running puffing-billy in a parallel test environment (for instance, in Rails 6), frequent crashes occur, with errors like the following:

123145443577856:error:0909006C:PEM routines:get_name:no start line:crypto/pem/pem_lib.c:745:Expecting: TRUSTED CERTIFICATE
123145443577856:error:140DC009:SSL routines:use_certificate_chain_file:PEM lib:ssl/ssl_rsa.c:622:
Assertion failed: (e > 0), function SslContext_t, file ssl.cpp, line 203.

123145562779648:error:0909006C:PEM routines:get_name:no start line:crypto/pem/pem_lib.c:745:Expecting: ANY PRIVATE KEY
123145562779648:error:140B0009:SSL routines:SSL_CTX_use_PrivateKey_file:PEM lib:ssl/ssl_rsa.c:556:
Assertion failed: (e > 0), function SslContext_t, file ssl.cpp, line 196.

These errors occur because parallel puffing-billy processes will overwrite each other's certificate files. To solve the problem, I've prefixed the certificate filenames with the current process id.

@ronwsmith

ronwsmith commented Jun 13, 2020

Copy link
Copy Markdown
Collaborator

Thanks for the contribution. Do you know if these files get cleaned up at some point automatically? I'd want to avoid the directory continually growing and potentially causing issues.

@clayton-shopify

clayton-shopify commented Jun 15, 2020

Copy link
Copy Markdown
Contributor Author

Do you know if these files get cleaned up at some point automatically?

Yes, because they're stored in the operating system's temporary folder, which is periodically cleaned up:

puffing-billy/lib/billy/config.rb

Line 39 in 16a90c0

@certs_path = File.join(Dir.tmpdir, 'puffing-billy', 'certs')

@ronwsmith
ronwsmith merged commit e16aad3 into oesmith:master Aug 27, 2020
@ronwsmith

ronwsmith commented Aug 27, 2020

Copy link
Copy Markdown
Collaborator

Released in 2.4.0

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@clayton-shopify @ronwsmith