-
Notifications
You must be signed in to change notification settings - Fork 438
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
Debugger: File breakpoint storage #887
Debugger: File breakpoint storage #887
Conversation
The breakpoint storage location needs to be the same between the agent and daemon.
If the breakpoint file is being written to it's ok to skip breakpoints for this request.
This may be helpful: https://github.com/GoogleCloudPlatform/google-cloud-php/tree/master/src/Core/Lock We could update the interface to pass in a flag to use either a blocking or non blocking lock |
Awesome, I'll take a look at those. |
Looks like I also need to add an option for the lock type. In this use case, it's fine to have many readers so we don't need to acquire an exclusive lock when reading. The new signature would either need to be:
edited: blocking defaults to true (unlike symfony) because that's the current behavior for |
Add options to LockInterface#acquire for blocking and exclusive locking options.
@dwsupplee Went with |
A cursory look at the second signature SGTM. I am away from my desk for a bit, will review this and #848 tonight :). |
src/Core/Lock/LockInterface.php
Outdated
@@ -25,10 +25,18 @@ | |||
/** | |||
* Acquires a lock that will block until released. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/Core/Lock/FlockLock.php
Outdated
* | ||
* @type bool $blocking Whether the process should block while waiting | ||
* to acquire the lock. **Defaults to** true. | ||
* @type bool $exclusive If true, acquire an excluse (write) lock. If |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
// Acquire an exclusive write lock (blocking). There should only be a | ||
// single Daemon that can call this. | ||
try { | ||
$this->lock->acquire(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -0,0 +1,48 @@ | |||
<?php | |||
/** | |||
* Copyright 2017 Google Inc. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really nice! One last little thing I noticed.
src/Core/Lock/LockInterface.php
Outdated
* | ||
* @type bool $blocking Whether the process should block while waiting | ||
* to acquire the lock. **Defaults to** true. | ||
* @type bool $exclusive If true, acquire an excluse (write) lock. If |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Adds a file-based implementation of
BreakpointStorageInterface
. This will allow windows users to use debugger.Note: we're using
flock
for attempting to make our file read/writes safe between 2 separate processes.