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.

Sign up for GitHub

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

Jump to bottom

Convert synchronized usages to AutoLock #5083

Closed
sbordet opened this issue Jul 27, 2020 · 3 comments · Fixed by #5084
Closed

Convert synchronized usages to AutoLock #5083

sbordet opened this issue Jul 27, 2020 · 3 comments · Fixed by #5084
Assignees
@sbordet
Projects
Jetty 10.0.0

Comments

@sbordet
Copy link
Contributor

sbordet commented Jul 27, 2020

Jetty version
10.0.x

Description
To be more Loom friendly we need the usages of synchronized to be converted to use java.util.locks.Lock or, better, Jetty's AutoLock.

This would allow early testing of Loom prototypes on a more fair basis.
@sbordet sbordet self-assigned this Jul 27, 2020
@sbordet sbordet added this to To Do in Jetty 10.0.0 via automation Jul 27, 2020
sbordet added a commit that referenced this issue Jul 27, 2020
@sbordet
Fixes #5083 - Convert synchronized usages to AutoLock.
8d69fc4 
* Replaced relevant usages of synchronized with AutoLock.
* Made AutoLock serializable since classes that use it may be stored in the HttpSession.
* Added convenience methods to AutoLock to execute lambdas with the lock held.
* Introduced AutoLock.WithCondition to use a Lock and a Condition together.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet linked a pull request Jul 27, 2020 that will close this issue
Fixes #5083 - Convert synchronized usages to AutoLock. #5084
Merged
@sbordet
Copy link
Contributor Author

sbordet commented Jul 27, 2020

This issue is under the Loom umbrella issue #5078.

@gregw
Copy link
Contributor

gregw commented Jul 28, 2020

But this change needs to stand on it's own merits rather than be justified by Loom.

@sbordet sbordet mentioned this issue Jul 28, 2020
Closed
@sbordet
Copy link
Contributor Author

sbordet commented Jul 28, 2020

@gregw this change has already improved locking in few places, clarified others and spun off #5081 and #5086.

Overall it is a good cleanup, but obviously getting rid of all synchronized is much more than just fixing a few things.
However, we would not have found the few things if we did not do this review 😃

This was referenced Jul 28, 2020
Closed
Closed
Closed
Open
Closed
Closed
sbordet added a commit that referenced this issue Jul 29, 2020
@sbordet
Fixes #5083 - Convert synchronized usages to AutoLock.
089e51f 
Updates after review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@joakime joakime moved this from To Do to In Progress in Jetty 10.0.0 Jul 30, 2020
sbordet added a commit that referenced this issue Aug 4, 2020
@sbordet
Issue #5083 - Convert synchronized usages to AutoLock.
edd6ca2 
Updates after review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Jetty 10.0.0 automation moved this from In Progress to Done Aug 4, 2020
sbordet added a commit that referenced this issue Aug 4, 2020
@sbordet
Merge pull request #5084 from eclipse/jetty-10.0.x-convert_synchroniz…
a79c0cf 
…ed_to_autolock

Fixes #5083 - Convert synchronized usages to AutoLock.
sbordet added a commit that referenced this issue Aug 4, 2020
@sbordet
Issue #5083 - Convert synchronized usages to AutoLock.
062878f 
Fixes after merge.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbordet sbordet

Labels
None yet
Projects
No open projects
Jetty 10.0.0
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #5083 - Convert synchronized usages to AutoLock. jetty/jetty.project
2 participants
@gregw @sbordet