Fix goroutine deadlock from unlocked mutex during panic

[fulghum]

Fixes a deadlock in DoltgresType.IoOutput/ConvertToType where a panic while resolving a type's cast or output function (e.g. an index-out-of-range on an unexpected empty ResolvedTypes()) left the type's mutex locked
forever, hanging every later query that touched the same type.

Also adds a step in the CI workflow to check for signs of a timeout in the logs, and if so, errors with a message about a deadlock timeout.

[github-actions]

	Main	PR
covering_index_scan_postgres	2138.67/s	2131.75/s	-0.4%
groupby_scan_postgres	137.56/s	139.43/s	+1.3%
index_join_postgres	693.66/s	695.32/s	+0.2%
index_join_scan_postgres	911.16/s	904.15/s	-0.8%
index_scan_postgres	33.34/s	32.29/s	-3.2%
oltp_delete_insert_postgres	834.67/s	845.41/s	+1.2%
oltp_insert	764.93/s	759.22/s	-0.8%
oltp_point_select	3597.09/s	3683.70/s	+2.4%
oltp_read_only	3531.40/s	3503.97/s	-0.8%
oltp_read_write	2519.18/s	2492.22/s	-1.1%
oltp_update_index	803.04/s	795.41/s	-1.0%
oltp_update_non_index	873.55/s	855.78/s	-2.1%
oltp_write_only	1827.05/s	1830.73/s	+0.2%
select_random_points	2065.30/s	2128.52/s	+3.0%
select_random_ranges	1636.52/s	1628.57/s	-0.5%
table_scan_postgres	32.84/s	32.26/s	-1.8%
types_delete_insert_postgres	851.76/s	837.74/s	-1.7%
types_table_scan_postgres	14.03/s	13.82/s	-1.5%

[itoqa]

Commit: e44c548: 15 test cases ran, 14 passed ✅, 1 additional finding ⚠️.

Summary

Coverage spans type conversion and output formatting across normal flows, cache reuse and refresh, concurrent callers, retries after errors, and recovery from panics. It also exercises edge cases involving special, composite, array, modified, serialized, and unknown values, with overall behavior appearing healthy aside from one stale-output edge case.

Safe to merge — the only failure is a medium-severity, unrelated pre-existing cache issue and is not attributable to this PR; no PR regressions or newly introduced failures were identified. It is a flag for later rather than a merge blocker.

Tests run by Ito

Result	Severity	Type	Description
✅	—	Cache	Several callers formatted values at the same time without hanging or returning different results. The local race-enabled Go suite passed, and the shared output function was resolved and published safely.
✅	—	Cache	A failed cast attempt does not block the next conversion. The next attempt can build the cast and finish successfully.
✅	—	Cache	Refreshing a type's output formatting did not break conversions or later variable substitutions. The local race-enabled Go type suite passed, and source inspection confirms the two caches are kept separate.
✅	—	Cache	Serialized values were deserialized, converted, and serialized again in the correct order. Errors from the first two steps were returned immediately, so a failed conversion did not continue or produce a misleading serialized value.
✅	—	Cast	The first conversion succeeds, stores the cast for the destination type, and returns the converted value.
✅	—	Cast	A failed cast lookup did not block later conversions. The next attempt resolved the cast and converted the value successfully.
✅	—	Cast	The first cast attempt panicked as expected, and the next conversion finished successfully instead of hanging.
✅	—	Cast	An unknown text value keeps its cached conversion result and is parsed by the target type instead of rebuilding the conversion.
✅	—	Cast	A failed cast lookup does not leave the shared type in a broken state. The next conversion can resolve and cache the valid cast, although the executable fixture was unavailable during this run.
✅	—	Cast	A resolver panic cannot leave later conversions stuck. The available source check confirms that the lock is released during panic cleanup and that incomplete cast data is not saved.
✅	—	Output	The first formatting call loaded the registered output function, and the second call reused it while producing the correct string.
✅	—	Output	Modifier-input, array, and composite values all used the current type when formatting output.
✅	—	Output	After the output function changed, repeated formatting used the replacement function and its refreshed metadata every time.
✅	—	Output	Concurrent callers finished safely, and later output used the replacement function instead of an older one.
⚠️		Output	The first formatting attempt panicked. After the function was corrected, the retry completed but still returned the old output instead of the corrected text.

Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Retry reuses stale output function

• Severity: Medium
• Description: The first formatting attempt panicked. After the function was corrected, the retry completed but still returned the old output instead of the corrected text.
• Impact: After a special value fails to resolve, a retry can return stale, incorrect output instead of the corrected result. The wrong output can persist in the cached type until the cache is cleared or the process is restarted.
• Steps to Reproduce:
  1. Create an array, composite, or modified type whose output function has an empty resolved-type list.
  2. Call the value formatting path and recover the index-out-of-range panic.
  3. Replace the registry function with a valid function while keeping the same output-function ID.
  4. Call the formatter again and compare the returned text with the corrected function's output.
• Stub / mock content: The test used isolated in-process Go QuickFunction and function-registry fixtures; no production database data, external service, or user credentials were used.
• Code Analysis: In server/types/type.go:752-764, getOrResolveOutFunc locks the type and enters the cache-miss branch at lines 755-756. It stores t.OutputFunc in t.outFuncID and publishes globalFunctionRegistry.GetFunction(ctx, t.OutputFunc) into t.outFunc at line 757 before the special-type setup is complete. For types with ModInFunc, IsArrayType, or IsCompositeType, line 759 reads ResolvedTypes and line 760 indexes resTypes[0]. An empty slice panics there. The deferred unlock added by this PR releases t.mutex, but it does not roll back t.outFunc or t.outFuncID. On retry, the condition at line 755 is false because the cached function is non-nil and its ID still equals OutputFunc, so the corrected registry function is never looked up. The smallest fix is to resolve and validate the special function using local variables, then publish t.outFunc and t.outFuncID only after the ResolvedTypes rewrite succeeds; alternatively, clear both cache fields when resolution panics.

Evidence Package

• text/OUTPUT-3/OUTPUT-3-stale-retry.txt — 3 KB

Tip

Reply with @itoqa to send us feedback on this test run.

[github-actions]

	Main	PR
Total	42090	42090
Successful	18949	18951
Failures	23141	23139
Partial Successes1	5338	5340

	Main	PR
Successful	45.0202%	45.0249%
Failures	54.9798%	54.9751%

${\color{red}Regressions (1)}$

copyselect

QUERY:          drop table test3;
RECEIVED ERROR: Doltgres sent additional messages after ReadyForQuery

${\color{lightgreen}Progressions (2)}$

indirect_toast

QUERY: INSERT INTO indtoasttest(descr, f1, f2) VALUES('one-toasted,one-null, via indirect', repeat('1234567890',30000), NULL);

subselect

QUERY: select count(*) from tenk1 t
where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0);

Footnotes

1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

[itoqa]

History reset (rebase or force-push detected). Starting test narrative over.

Commit: 8d53ac2: 14 test cases ran, 14 passed ✅.

Summary

Coverage spans normal and mixed-type value conversion, repeated and concurrent operations, session isolation, consistent array and composite value formatting, recovery after malformed inputs, and formatter replacement behavior. It also exercises adversarial error and timeout handling, including distinguishing real deadlocks from ordinary failures and incomplete diagnostic conditions.

Safe to merge — the run found no PR-attributable regressions or new failures across the covered conversion, output, recovery, concurrency, and timeout-handling behaviors. The formatter-replacement scenario could not be fully rerun because its local target stopped, but source review supports the expected cache-reset behavior and this is a flag for later rather than a merge blocker.

Tests run by Ito

Result	Severity	Type	Description
✅	—	Cast	Two inserts converted the string value '7' to integer 7, and selecting the temporary table returned 7 for both rows.
✅	—	Cast	An invalid integer conversion returned the expected error, and the next conversion completed normally with the value 8.
✅	—	Cast	The first invalid integer conversion returned an error, then an immediate retry returned 9 and the next conversion returned 10 without restarting the server or timing out.
✅	—	Cast	Sixteen sessions converted the same value at once, and every session returned 11. A follow-up conversion also returned 11 without timing out.
✅	—	Conversion	Assignments, explicit casts, and implicit mixed-value conversions all returned the expected values.
✅	—	Conversion	Two separate sessions converted their temporary-table values correctly, and repeating a conversion in either session returned the expected result without a cross-session error.
✅	—	Output	Array values returned {1,2,3}, and composite values returned (left,right) on repeated reads. The client-visible text stayed the same each time.
✅	—	Output	A malformed output definition caused the expected first-call error, then an immediate retry completed and returned the recovered value. The retry did not hang, so later output requests remained available.
✅	—	Output	The requested replacement could not run because the local database target stopped before the retest. Source inspection shows that changing the output function identity clears the cached formatter and rebuilds it for the current type.
✅	—	Regression	A regression log with the exact timeout message made the pull request job fail clearly, while the diagnostic log stayed available for upload.
✅	—	Regression	When the main regression log contains the exact timeout marker, the post-check fails the job and keeps the diagnostic log available for upload.
✅	—	Regression	An ordinary failing test did not trigger the deadlock check. The regression job continued with exit 0, kept both tracker files available, and left the diagnostic log readable.
✅	—	Regression	Missing, empty, and differently worded timeout logs followed the normal check without crashing or raising a false deadlock failure. Available logs stayed observable, and missing-file diagnostics were identified.
✅	—	Regression	The CI checks failed on the exact timeout marker, ignored ordinary and ambiguous log text, and kept the available logs and tracker diagnostics for review.

Tip

Reply with @itoqa to send us feedback on this test run.

[zachmu]

LGTM!