UniqueLock.lock & SharedLock.lock are dangerous

[samchon]

Summary

• TSTL Version: 2.2.1
• Expected behavior: _SafeLock.lock() holds locker until be free
• Actual behavior: _SafeLock.lock() may not hold the critical section
• Solution: _SafeLock.lock() must have its identical implementation code.

Code occuring the danger

• UniqueLock.lock()
• SharedLock.lock()
• base._SafeLock

Detailed Story

UniqueLock.lock() and SharedLock.lock() are inline functions of base._SafeLock.lock() and the base._SafeLock.lock() is an inline function of base._SafeLock.try_lock(), too. It seems dangerous implementation considering characteristics of the final destination function base._SafeLock.try_lock()

• UniqueLock.lock() -> base._SafeyLock.lock() -> base._SafeLock.try_lock()
• SharedLock.lock() -> base._SafeyLock.lock() -> base._SafeLock.try_lock()

tstl/src/base/thread/_SafeLock.ts

Lines 19 to 29 in 3c47bbd

	export async function try_lock
	(
	locker: () => Promise<boolean>,
	unlocker: () => Promise<void>,
	lambda: () => void | Promise<void>
	): Promise<boolean>
	{
	// TRY LOCK
	let ret: boolean = await locker();
	if (ret === false)
	return false;

The base._SafeLock.try_lock() function skips calling lambda if locker returns false. The UniqueLock.lock() and SharedLock.lock() functions, they must hold critical section (locker) until be free. However, they're referencing base._SafeLock.try_lock(), who does not ensure holding critical section in always.

interface ILockable
{
    lock(): Promise<void>;
    unlock(): Promise<void>;
}
interface ISharedLockable
{
    lock_shared(): Promise<void>;
    unlock_shared(): Promise<void>;
}

Only in TypeScript level, it's not danger because the parametric function (locker) type must return Promise<void>. However, let's imagine an extra-ordinary case. There's an user does not use TypeScript and wants to use a custom class. Even the custom class does not follow definition of ILockable (or ISharedLockable) and returns Promise. If the custom class returns false, holding critical section in always is not possible.

Conclusion

To avoid such danger, _SafeLock.lock() must have its identical implementation code.