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

Remove GOTO and prevent deadlock. #28

Closed
wants to merge 1 commit into from
Closed

Remove GOTO and prevent deadlock. #28

wants to merge 1 commit into from

Conversation

bweston92
Copy link
Member

Different syntax to remove GOTO and also made sure a release always happens after a lock to prevent deadlock.

@oqq
Copy link
Member

oqq commented Jan 24, 2017

puh.. i dont like this huge try/catch block around all code.
Maybe an event listener with a cleanup task would be a better solution?

see http://symfony.com/doc/current/components/console/events.html

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.06%) to 33.533% when pulling de8bc47 on bweston92:commands into c63f123 on prooph:commands.

@bweston92 bweston92 changed the base branch from commands to master January 24, 2017 19:59
@oqq
Copy link
Member

oqq commented Jan 24, 2017

I have found this symphony doc:
http://symfony.com/doc/master/console/lockable_trait.html

if not released explicitly, Symfony releases the lock
automatically when the execution of the command ends

So my suggestion is to remove all release() requests and let symphony do it automatically. If it is not working, we should create an symphony issue. ;)

Never mind .. symphony code say this is a lie

@bweston92
Copy link
Member Author

Closing as this would be very hard to come across and again would be done to users implementation.

If a user ran a web server with ReactHTTP (doesn't end process so flock doesn't free).
The server had an endpoint to add services that called the command, but during the lock and release an exception is thrown then the lock would continue to be locked until the process itself dies.

@bweston92 bweston92 closed this Jan 24, 2017
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.

None yet

3 participants