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

Use ReentrantLocks instead of synchronized blocks #184

Merged
merged 3 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -81,27 +81,27 @@ public MapperConfiguratorBase(MAPPER mapper, Annotations[] defaultAnnotations)
/***********************************************************
*/

public synchronized final void setMapper(MAPPER m) {
public final void setMapper(MAPPER m) {
_mapper = m;
}

public synchronized final void setAnnotationsToUse(Annotations[] annotationsToUse) {
public final void setAnnotationsToUse(Annotations[] annotationsToUse) {
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
_setAnnotations(mapper(), annotationsToUse);
}

public synchronized final void configure(DeserializationFeature f, boolean state) {
public final void configure(DeserializationFeature f, boolean state) {
mapper().configure(f, state);
}

public synchronized final void configure(SerializationFeature f, boolean state) {
public final void configure(SerializationFeature f, boolean state) {
mapper().configure(f, state);
}

public synchronized final void configure(JsonParser.Feature f, boolean state) {
public final void configure(JsonParser.Feature f, boolean state) {
mapper().configure(f, state);
}

public synchronized final void configure(JsonGenerator.Feature f, boolean state) {
public final void configure(JsonGenerator.Feature f, boolean state) {
mapper().configure(f, state);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.fasterxml.jackson.jaxrs.cbor;

import java.util.*;
import java.util.concurrent.locks.ReentrantLock;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
Expand All @@ -17,6 +18,9 @@
public class CBORMapperConfigurator
extends MapperConfiguratorBase<CBORMapperConfigurator, ObjectMapper>
{
// @since 2.18
private final ReentrantLock _lock = new ReentrantLock();
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved

/*
/**********************************************************
/* Construction
Expand All @@ -32,18 +36,24 @@ public CBORMapperConfigurator(ObjectMapper mapper, Annotations[] defAnnotations)
* Method that locates, configures and returns {@link ObjectMapper} to use
*/
@Override
public synchronized ObjectMapper getConfiguredMapper() {
/* important: should NOT call mapper(); needs to return null
* if no instance has been passed or constructed
*/
public ObjectMapper getConfiguredMapper() {
// important: should NOT call mapper(); needs to return null
// if no instance has been passed or constructed
return _mapper;
}

@Override
public synchronized ObjectMapper getDefaultMapper() {
public ObjectMapper getDefaultMapper() {
if (_defaultMapper == null) {
_defaultMapper = new ObjectMapper(new CBORFactory());
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_defaultMapper == null) {
_defaultMapper = new ObjectMapper(new CBORFactory());
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _defaultMapper;
}
Expand All @@ -63,8 +73,15 @@ public synchronized ObjectMapper getDefaultMapper() {
protected ObjectMapper mapper()
{
if (_mapper == null) {
_mapper = new ObjectMapper(new CBORFactory());
_setAnnotations(_mapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_mapper == null) {
_mapper = new ObjectMapper(new CBORFactory());
_setAnnotations(_mapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _mapper;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.fasterxml.jackson.jaxrs.json;

import java.util.*;
import java.util.concurrent.locks.ReentrantLock;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
Expand All @@ -16,6 +17,9 @@
public class JsonMapperConfigurator
extends MapperConfiguratorBase<JsonMapperConfigurator, ObjectMapper>
{
// @since 2.18
private final ReentrantLock _lock = new ReentrantLock();

/*
/**********************************************************
/* Construction
Expand All @@ -31,18 +35,24 @@ public JsonMapperConfigurator(ObjectMapper mapper, Annotations[] defAnnotations)
* Method that locates, configures and returns {@link ObjectMapper} to use
*/
@Override
public synchronized ObjectMapper getConfiguredMapper() {
/* important: should NOT call mapper(); needs to return null
* if no instance has been passed or constructed
*/
public ObjectMapper getConfiguredMapper() {
// important: should NOT call mapper(); needs to return null
// if no instance has been passed or constructed
return _mapper;
}

@Override
public synchronized ObjectMapper getDefaultMapper() {
public ObjectMapper getDefaultMapper() {
if (_defaultMapper == null) {
_defaultMapper = new ObjectMapper();
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_defaultMapper == null) {
_defaultMapper = new ObjectMapper();
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _defaultMapper;
}
Expand All @@ -62,8 +72,15 @@ public synchronized ObjectMapper getDefaultMapper() {
protected ObjectMapper mapper()
{
if (_mapper == null) {
_mapper = new ObjectMapper();
_setAnnotations(_mapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_mapper == null) {
_mapper = new ObjectMapper();
_setAnnotations(_mapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _mapper;
}
Expand Down
2 changes: 2 additions & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ PJ Fanning (@pjfanning)
* Contributed #166: `ProviderBase` class shows contention on synchronized
block using `LRUMap` _writers instance
(2.14.2)
* Contributed #184: Use `ReentrantLock`s instead of synchronized blocks
(2.18.0)

Steven Schlansker (@stevenschlansker)

Expand Down
3 changes: 2 additions & 1 deletion release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ Sub-modules:

2.18.0 (not yet released)

No changes since 2.17
#184: Use `ReentrantLock`s instead of synchronized blocks
(contributed by @pjfanning)

2.17.0 (12-Mar-2024)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.fasterxml.jackson.jaxrs.smile;

import java.util.*;
import java.util.concurrent.locks.ReentrantLock;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
Expand All @@ -18,6 +19,9 @@
public class SmileMapperConfigurator
extends MapperConfiguratorBase<SmileMapperConfigurator, ObjectMapper>
{
// @since 2.18
private final ReentrantLock _lock = new ReentrantLock();

/*
/**********************************************************
/* Construction
Expand All @@ -33,18 +37,24 @@ public SmileMapperConfigurator(ObjectMapper mapper, Annotations[] defAnnotations
* Method that locates, configures and returns {@link ObjectMapper} to use
*/
@Override
public synchronized ObjectMapper getConfiguredMapper() {
/* important: should NOT call mapper(); needs to return null
* if no instance has been passed or constructed
*/
public ObjectMapper getConfiguredMapper() {
// important: should NOT call mapper(); needs to return null
// if no instance has been passed or constructed
return _mapper;
}

@Override
public synchronized ObjectMapper getDefaultMapper() {
public ObjectMapper getDefaultMapper() {
if (_defaultMapper == null) {
_defaultMapper = new ObjectMapper(new SmileFactory());
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_defaultMapper == null) {
_defaultMapper = new ObjectMapper(new SmileFactory());
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _defaultMapper;
}
Expand All @@ -64,8 +74,15 @@ public synchronized ObjectMapper getDefaultMapper() {
protected ObjectMapper mapper()
{
if (_mapper == null) {
_mapper = new ObjectMapper(new SmileFactory());
_setAnnotations(_mapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_mapper == null) {
_mapper = new ObjectMapper(new SmileFactory());
_setAnnotations(_mapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _mapper;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.fasterxml.jackson.jaxrs.xml;

import java.util.*;
import java.util.concurrent.locks.ReentrantLock;

import com.fasterxml.jackson.databind.*;

Expand All @@ -20,6 +21,9 @@
public class XMLMapperConfigurator
extends MapperConfiguratorBase<XMLMapperConfigurator, XmlMapper>
{
// @since 2.18
private final ReentrantLock _lock = new ReentrantLock();

/*
/**********************************************************
/* Construction
Expand All @@ -35,21 +39,27 @@ public XMLMapperConfigurator(XmlMapper mapper, Annotations[] defAnnotations)
* Method that locates, configures and returns {@link XmlMapper} to use
*/
@Override
public synchronized XmlMapper getConfiguredMapper() {
/* important: should NOT call mapper(); needs to return null
* if no instance has been passed or constructed
*/
public XmlMapper getConfiguredMapper() {
// important: should NOT call mapper(); needs to return null
// if no instance has been passed or constructed
return _mapper;
}

@Override
public synchronized XmlMapper getDefaultMapper()
public XmlMapper getDefaultMapper()
{
if (_defaultMapper == null) {
// 10-Oct-2012, tatu: Better do things explicitly...
JacksonXmlModule module = getConfiguredModule();
_defaultMapper = (module == null) ? new XmlMapper() : new XmlMapper(module);
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_defaultMapper == null) {
// 10-Oct-2012, tatu: Better do things explicitly...
JacksonXmlModule module = getConfiguredModule();
_defaultMapper = (module == null) ? new XmlMapper() : new XmlMapper(module);
_setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _defaultMapper;
}
Expand All @@ -74,8 +84,15 @@ protected JacksonXmlModule getConfiguredModule()
protected XmlMapper mapper()
{
if (_mapper == null) {
_mapper = new XmlMapper();
_setAnnotations(_mapper, _defaultAnnotationsToUse);
_lock.lock();
try {
if (_mapper == null) {
_mapper = new XmlMapper();
_setAnnotations(_mapper, _defaultAnnotationsToUse);
}
} finally {
_lock.unlock();
}
}
return _mapper;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
* mapper to use can be configured in multiple ways:
* <ul>
* <li>By explicitly passing mapper to use in constructor
* <li>By explcitly setting mapper to use by {@link #setMapper}
* <li>By explicitly setting mapper to use by {@link #setMapper}
* <li>By defining JAX-RS <code>Provider</code> that returns {@link YAMLMapper}s.
* <li>By doing none of above, in which case a default mapper instance is
* constructed (and configured if configuration methods are called)
Expand Down
Loading
Loading