Just like in Java, you syncronize on the value, `t...

# announcements

[karelpeeters]

Just like in Java, you syncronize on the value, true or false, not on the variable. For this reason it's a really bad idea to ever use a primitive for synchronization.

[janvladimirmostert]

ok, that's exactly the reason I want to use primitives, I have one scenario where optimistic locking (in DB) is not going to work for us, so I want to synchronize on the entity's id in the short term until we can implement a better solution for it.
Obviously this will only work as long as we're only running one instance of the application.

[karelpeeters]

This is still a terrible design idea. What if anyone else in the whole application also decides to syncronize on Int for something completely different? You get nearly unreproducable deadlocks.

[karelpeeters]

Just write a data class ID(val id: Int) and syncronize on that if you really need to.

[janvladimirmostert]

@karelpeeters yes, it's a temporary hack for which we'll implement a proper fix later on. We store trees in our DB and we have logic that makes sure that somebody doesn't store circular references. Since this logic is in code and not enforced on the DB, it's possible that two people can add nodes at the same time which might cause circular references.

As a temporary hack, we're going to do synchronize("project-344") { }

Once we need to scale to more than one instance, we'll obviously need to re-visit this and implement a better solution

[karelpeeters]

Syncronizing on String won't work either, there's no guarantee two equal strings will be the same instance, see the common "why doesn't "test" == String("test") work?".

[karelpeeters]

Is it so difficult to use a global val lock = Object() lock?

[janvladimirmostert]

@karelpeeters that's what I'm currently doing, discovered the hard way that strings doesn't synchronize

[janvladimirmostert]

hopefully I can rip out this code in the near future, feels very hacky

[janvladimirmostert]

object Lock {
	val projectMap = ConcurrentHashMap<Long, ProjectVO>()
}

// synchronize on project id
inline fun <R> projectLock(projectId: Long, block: () -> R): R =
	synchronized(Lock.projectMap.putIfAbsent(projectId, ProjectVO()) ?: ProjectVO()) {
		block()
	}

That works

[karelpeeters]

And why doesn't you database object have an internal lock?

[janvladimirmostert]

In a simple scenario:

User1 is looking at A
User2 is looking at B

User1 adds B as a child of A
User2 adds A as a child of B

If this happens at the same time, you have a circular dependency A->B->A->B

This is also in a multi tenancy system, so 50 users could be trying to add B to A while 50 other users could be adding B to A

The probability of it happening is small, but we'd rather have a lock in place to prevent a circular references

We'll use an extern Redis / Hazelcast instance at some point to coordinate this lock in the future, for now a synchronize is fine while we're running a single instance.

[karelpeeters]

And syncronizing the addChild of your DB on a single childLock is nog enough?

[janvladimirmostert]

on which child do you lock, some of these trees have 1000s of children and some of the children are in 1000s of other trees. So we lock at a project level, typically you have at most 5 people on a project, so a lock won't cause a lot of contention.

[karelpeeters]

You don't lock on a child, you lock on a global lock that's in the database.

[janvladimirmostert]

yeah, we're planning to do that, but we don't have Hazelcast and we're only adding Hazelcast caching once we're running more than one instance, so until then, this temporary lock solution should work fine, after that we're planning to use a Hazelcast distributed lock.
DB lock is also an option.

[karelpeeters]

Okay, good luck!

[janvladimirmostert]

thanks for the help Karel!