Problems with simultaneous use of thread-local variables and object caching
If reproduced, please name the source:/funnyzpc/p/18313879
ahead
Some time ago, I read a piece of code someone wrote about locking (object caching + thread-local variables), which roughly describes a function like this:
External pass a key, you need to go to the global variable according to the key to find whether there is, if there is, it means that someone on the key to lock, down the specific business code will not be executed, at the same time, at the same time Oh To determine whether the key is held by the current thread, if it is not held by the current thread can not be down the execution of business code ~
And then you have to release the key lock when the business code finishes executing, i.e., you have to release the key lock from theThreadLocal
Remove the key.
Of course the need goes beyond that, it's the specificity of the business that requiresThreadLocal
holds several different keys at the same time, which suggests that theThreadLocal
The generic type is definitely a List or Set.
Then say the code, in order to demonstrate the problem code written more abbreviated, the following I then one by one to illustrate the possible problems 🎈
basic logic
The function contains roughly two functions: the
-
lock
: The main purpose is to find out if the public cache and thread local variables contain the specified key passed in, if not, then try to write to the global variables and theThreadLocal
and returns true to indicate that the lock was acquired. -
release
: This is called after the business logic has been processed, and this function mainly does global caching as well as theThreadLocal
Remove the key and return the status (true/false). -
contains
: public method for use by the above two methods, logic: determine whether a global variable or aThreadLocal
There is a specified key in it, and this method uses theprivate
qualify or modify (grammar)
Okay, ready to see the code 😂
Let's start with the first edition
- coding
public class CacheObjectLock {
// Global Object Cache
private static List<Object> GLOBAL_CACHE = new ArrayList<Object>(8);
// thread-local variable
private static ThreadLocal<List<Object>> THREAD_CACHE = new ThreadLocal<List<Object>>();
// Trying to add a lock
public synchronized boolean lock(Object obj){
if((obj)){
return false;
}
List al = null;
if((al=THREAD_CACHE.get())==null){
al = new ArrayList(2);
THREAD_CACHE.set(al);
}
(obj);
GLOBAL_CACHE.add(obj);
return true;
}
// Determine the presence or absence ofkey
public boolean contains(Object obj){
List<Object> objs;
return GLOBAL_CACHE.contains(obj)?true:(objs=THREAD_CACHE.get())==null?false:(obj);
}
// liberate (a *er)keypadlock,Same as above lock method correspondence
public boolean release(Object obj){
if( (obj) ){
List<Object> objs = THREAD_CACHE.get();
if(null!=objs){
(obj);
GLOBAL_CACHE.remove(obj);
}
return true;
}
return false;
}
}
-
test code
Because it's a lock, you have to use a multi-threaded test, and here I simply use the
parallel stream
+ Multiple loops to test.
public class CacheObjectLockTest {
private CacheObjectLock LOCK = new CacheObjectLock();
public void test1(){
(0,10000).parallel().forEach(i->{
if(i%3==0){
i-=2;
}
Boolean b = null;
if((b=(i))==false ){
return ;
}
Boolean c = null;
try {
// do something ...
// (1);
} catch (Exception e) {
throw new RuntimeException(e);
}finally {
c = (i);
}
if(b!=c){
("b:"+b+" c:"+c+" => "+().getName());
}
});
// (9);
}
@Test
public void test2(){
for(int i=0;i<10;i++){
this.test1();
}
}
}
-
Test results
-
analyze
Obviously, this is not the right
release
Adding locks causes it, actually, that's not accurate...
First of all, it's important to understandlock
plussynchronized
The scope of the synchronization lock is for the current instance of therelease
It's not added.synchronized
So.release
It's ignorance.lock
plussynchronized
Take a closer look.GLOBAL_CACHE
What is it?ArrayList
I see.ArrayList
is not thread-safe becausesynchronized
The range is justlock
Inside this function, you can see from the test code that the(i)
From the beginning until(i)
There is no synchronization lock added in between, so by the time the(i)
From the beginning until(i)
There is thread competition in the middle of this, which happens to run into theArrayList
This insecurity will naturally throw the wrong way!
Because of the existence of unsafe classes, it is reasonable to suspect that theTHREAD_CACHE
is also subject to a multithreading exception because it this generic is alsoArrayList
!
Re-reading the second edition
Well, understanding what the problem is, naturally the solution is very easy:
- exist
release
method by adding thesynchronized
Statement. This is simple and brutal. - separate opinion
(obj);
as well asGLOBAL_CACHE.remove(obj);
Add synchronized locks for finer granularity
on account ofsynchronized
is write-exclusive, so there is no need for thecontains
separate lock in the middle
- Code (here only
release
Change)
public synchronized boolean release(Object obj){
if( (obj) ){
List<Object> objs = THREAD_CACHE.get();
if(null!=objs){
// synchronized (objs){
(obj);
// }
// synchronized (GLOBAL_CACHE){
GLOBAL_CACHE.remove(obj);
// }
}
return true;
}
return false;
}
- Test results
-
analyze
😂
Tested multiple rounds with success and nothing out of the ordinary, does that mean there must be nothing out of the ordinary????
No, no, no.
To make the problem clearer, change the test case and put thecontains
method is set topublic
, and then test cases.
public class CacheObjectLockTest {
private CacheObjectLock2 LOCK = new CacheObjectLock2();
public void test1(){
(0,10000).parallel().forEach(i->{
// String it = "K"+i;
if(i%3==0){
i-=2;
}
Boolean b = null;
if((b=(i))==false ){
return ;
}
Boolean c = null;
try {
// do something ...
// (1);
} catch (Exception e) {
throw new RuntimeException(e);
}finally {
c = (i);
}
if(b!=c){
("b:"+b+" c:"+c+" => "+().getName());
}
});
(9);
}
@Test
public void test2(){
for(int i=0;i<10;i++){
this.test1();
}
}
}
Put a break in the line(9);
Then step by step into theThreadLocal
(used form a nominal expression)get()
Methods in:
See, even though the key has been removed.ThreadLocal
It is associated with the outer layer of the keyArrayList
, because the development machines are all better configured, once you cause theThreadLocal
OOM is an inevitable consequence of this!
We know.ThreadLocal
It is a basic feature of the threads that it stores all the information of the respective threads separately according to the threads.set
Incoming objects that do not call theirremove
method, the variable will always existThreadLocal
this onemap
Middle.
If the above test code is managed in a thread pool, the thread pool will increase or decrease the number of threads according to the load, and if the threads used for each execution of the above code are not fixed, then the thread pool will not be used for the test code.ThreadLocal
Definitely leads to jvm OOM 😂
It's likejava
Inside the file read and write.open
And after that, you have to, you have to.close
Operation.
final change
- coding
public class CacheObjectLock3 {
private static List<Object> GLOBAL_CACHE = new ArrayList<Object>(8);
private static ThreadLocal<List<Object>> THREAD_CACHE = new ThreadLocal<List<Object>>();
public synchronized boolean lock(Object obj){
if((obj)){
return false;
}
List al = null;
if((al=THREAD_CACHE.get())==null){
al = new ArrayList(2);
THREAD_CACHE.set(al);
}
(obj);
GLOBAL_CACHE.add(obj);
return true;
}
public boolean contains(Object obj){
List<Object> objs;
return GLOBAL_CACHE.contains(obj)?true:(objs=THREAD_CACHE.get())==null?false:(obj);
}
public synchronized boolean release(Object obj){
if( (obj) ){
List<Object> objs = THREAD_CACHE.get();
if(null!=objs){
// synchronized (objs){
(obj);
if(()){
THREAD_CACHE.remove();
}
// }
// synchronized (GLOBAL_CACHE){
GLOBAL_CACHE.remove(obj);
// }
}
return true;
}
return false;
}
}
- Test results
(Screenshots omitted)
Testing ok. Passed.
ultimate
The above code may not be perfect, but at least it sees the problem, especially using thepadlock
maybeThreadLocal
Be careful when you're in the middle of something.
The core code is only partially intercepted, if there is a problem please let me know, thank you ♥!