[kbkpbot]

Feature request by #23995

Note: This a generic connection pool implementation for database connection, network connection....
To support database pool, it need further enhance in db.mysql or other database, to support the ConnectionPoolable interface.

// ConnectionPoolable defines the interface for connection objects
pub interface ConnectionPoolable {
mut:
	// validate_conn checks if the connection is still usable
	validate_conn() !bool
	// close_conn terminates the physical connection
	close_conn() !
	// reset_conn returns the connection to initial state for reuse
	reset_conn() !
}

This module provides a robust connection pooling implementation for
managing reusable resources like database connections. It handles
connection lifecycle, validation, and efficient resource allocation
with minimal overhead.

[huly-for-github]

Connected to Huly®: V_0.6-23020

[gechandesu]

I suggest rename the ConnectionPoolable methods, IMO the struct name (that implements interface) should make it clear that the action is performed on the connection. Using the implements keyword will make this even more explicit.

validate_conn() !bool  -> validate() !bool
close_conn() !         -> close() !
reset_conn() !         -> reset() !

[kbkpbot]

I suggest rename the ConnectionPoolable methods, IMO the struct name (that implements interface) should make it clear that the action is performed on the connection. Using the implements keyword will make this even more explicit.

validate_conn() !bool  -> validate() !bool
close_conn() !         -> close() !
reset_conn() !         -> reset() !

Yes, this may simplify the interface methods' name.
For the close() method, many databases already defined it, but with different function signatures.
For example, in db.sqlite, it already defined with pub fn (mut db DB) close() !bool, but in db.mysql, it is defined with pub fn (mut db DB) close().

[einar-hjortdal]

That ConnectionPool looks so complex to me

[enghitalo]

Do we have any way to test race conditions?

[enghitalo]

Please, use time.utc instead of time.now, for performance reasons. Is that ok?

[kbkpbot]

That ConnectionPool looks so complex to me

Yes, this is a little complex :)
But we can have a start point for future connection pool developing....

[kbkpbot]

Do we have any way to test race conditions?

1. I ask AI to check for race condition..... (Indeed it can solve some bugs...)
2. In the test file, we can loop for many times to ensure some function work as expected.

[kbkpbot]

Please, use time.utc instead of time.now, for performance reasons. Is that ok?

simply replace .now() with .utc() will cause RUNTIME ERROR... I will check it ...

$ v test connection_test.v
---- Testing... ----------------------------------------------------------------------------------------------------------------
 FAIL     3.731 ms /media/HD/github/kbkpbot/v/vlib/pool/connection_test.v
/media/HD/github/kbkpbot/v/vlib/pool/connection_test.v:113: fn test_pool_exhaustion
    assert is_same_conn(c3, c2)

0x714e43988d41: at ???: RUNTIME ERROR: invalid memory access
/tmp/v_1000/tsession_7a682ea6c740_01JX7ZA1TJH5YFZB6DWS08B6BG/connection_test.01JX7ZA1TN571RYAT7G097:6962: by array_push
/tmp/v_1000/tsession_7a682ea6c740_01JX7ZA1TJH5YFZB6DWS08B6BG/connection_test.01JX7ZA1TN571RYAT7G097:4853: by anon_fn_7da8c9bc3f01925f___result_pool__ConnectionPoolable_729
/tmp/v_1000/tsession_7a682ea6c740_01JX7ZA1TJH5YFZB6DWS08B6BG/connection_test.01JX7ZA1TN571RYAT7G097:12441: by pool__ConnectionPool_create_conn_with_retry
/tmp/v_1000/tsession_7a682ea6c740_01JX7ZA1TJH5YFZB6DWS08B6BG/connection_test.01JX7ZA1TN571RYAT7G097:12976: by pool__ConnectionPool_prune_connections
/tmp/v_1000/tsession_7a682ea6c740_01JX7ZA1TJH5YFZB6DWS08B6BG/connection_test.01JX7ZA1TN571RYAT7G097:12916: by pool__ConnectionPool_background_maintenance
/tmp/v_1000/tsession_7a682ea6c740_01JX7ZA1TJH5YFZB6DWS08B6BG/connection_test.01JX7ZA1TN571RYAT7G097:4834: by pool__ConnectionPool_background_maintenance_thread_wrapper
0x0065ed4f: by ???
0xed9f4de977d88290: by ???
0x00000000: by ???
--------------------------------------------------------------------------------------------------------------------------------
To reproduce just failure 1 run:    '/media/HD/github/kbkpbot/v/v'  '/media/HD/github/kbkpbot/v/vlib/pool/connection_test.v'
Summary for all V _test.v files: 1 failed, 1 total. Elapsed time: 234 ms, on 1 job. Comptime: 229 ms. Runtime: 2 ms.

[kbkpbot]

Please don't merge this. As I found on tcc/windows, it may cause program hung sometimes. It may related to some sync/atomic bugs.

[enghitalo]

Do we have any way to test race conditions?

1. I ask AI to check for race condition..... (Indeed it can solve some bugs...)
2. In the test file, we can loop for many times to ensure some function work as expected.

Usually, valgrid is used to do these tests. I saw that there are some tests using valgrid in V. Maybe @spytheman can help us with that.

[spytheman]

Should this be rebased on master, now that a9f4a94 is merged, or are there other things that will need to be fixed before that?

[kbkpbot]

Should this be rebased on master, now that a9f4a94 is merged, or are there other things that will need to be fixed before that?

Just wait sometime, as atomic still has some bugs.
for example, in v/thirdparty/stdatomic/nix/atomic.h, __atomic_compare_exchange_2 has a prototype as _Bool __atomic_compare_exchange_2(unsigned short* x, unsigned short* expected, unsigned short y, int mo, int mo2); which is different to gcc's bool __atomic_compare_exchange_n (type *ptr, type *expected, type desired, bool weak, int success_memorder, int failure_memorder) , miss the weak parameter. See https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

I am working on a tcc commit which will implement these atomic builtins in assembly.

[kbkpbot]

MSVC generated code has bug...

mut conn := p.get()! as MockConn

will generated code twice:

	_result_pool__ConnectionPoolable_ptr _t5 = pool__ConnectionPool_get(p);
	if (_t5.is_error) {
		main__TestRunner_name_table[test_runner._typ]._method_fn_error(test_runner._object, 273, tos3("connection_test.v"), tos3("main"), tos3("test_error_handling"), IError_name_table[_t5.err._typ]._method_msg(_t5.err._object) );
		longjmp(g_jump_buffer, 1);
	}
	_result_pool__ConnectionPoolable_ptr _t6 = pool__ConnectionPool_get(p);
	if (_t6.is_error) {
		main__TestRunner_name_table[test_runner._typ]._method_fn_error(test_runner._object, 273, tos3("connection_test.v"), tos3("main"), tos3("test_error_handling"), IError_name_table[_t6.err._typ]._method_msg(_t6.err._object) );
		longjmp(g_jump_buffer, 1);
	}
	
 	
 	main__MockConn *conn = HEAP(main__MockConn, (*(main__MockConn*)__as_cast(((*(pool__ConnectionPoolable**)_t5.data))->_main__MockConn,v_typeof_interface_idx_pool__ConnectionPoolable(((*(pool__ConnectionPoolable**)_t6.data))->_typ), 133)));

and this cause assert fail

[spytheman]

MSVC generated code has bug...

mut conn := p.get()! as MockConn

try simplifying it to:

mut tconn := p.get()!
mut conn  := tconn as MockConn

to see if it will trigger a non buggy codegen in msvc

[spytheman]

Oh, you already did.

[spytheman]

The CI passed now for msvc too after the second retry.
The first try was https://github.com/vlang/v/actions/runs/15674428897/job/44151427768 where connection_test.v timed out after ~1 hour of waiting.
Perhaps there is a deadlock somewhere?

[kbkpbot]

The CI passed now for msvc too after the second retry. The first try was https://github.com/vlang/v/actions/runs/15674428897/job/44151427768 where connection_test.v timed out after ~1 hour of waiting. Perhaps there is a deadlock somewhere?

Is msvc can be skipped right now? I think it need some more work debugging the sync problem.
Use // vtest build: !msvc at the test file beginning?

[spytheman]

Yes, it can be skipped with // vtest build: !msvc .

[spytheman]

It timed out for a tcc-windows job on master too:
https://github.com/vlang/v/actions/runs/15737512081/job/44354089208 .

I'll see how it will behave in the coming days, it may need a !windows build tag 🤔 for now.

[spytheman]

It fails sporadically for gcc-windows too: https://github.com/vlang/v/actions/runs/15751959355/attempts/1 and https://github.com/vlang/v/actions/runs/15751959355/attempts/2 . It also failed on the PR that had the WaitGroup fix https://github.com/vlang/v/actions/runs/15752112204/job/44399224811 the first time.

I'll change the build tag to !windows to prevent further false positives for unrelated PRs, until it is fixed for good.