Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgen: fix windows hot reload #23350

Merged
merged 16 commits into from
Jan 3, 2025
Merged

Conversation

kbkpbot
Copy link
Contributor

@kbkpbot kbkpbot commented Jan 3, 2025

Fix issue #23214

Force generate _vinit_caller, _vcleanup_caller , these are needed under Windows, because dl.open() / dl.close() will call them when loading/unloading shared dll.

Huly®: V_0.6-21781

@kbkpbot
Copy link
Contributor Author

kbkpbot commented Jan 3, 2025

This PR will make -live work under Windows/TCC

D:\v\v\v\examples\hot_reload>v -live run message.v
OK reloads:    0 | Total reloads:    0 | Hello! Modify this message while the program is running.
OK reloads: 1869376609 | Total reloads: 544501614 | Hello! Modify this message while the program is running.
OK reloads: 1869376609 | Total reloads: 544501614 | Hello! Modify this message while the program is running.
OK reloads: 1869376609 | Total reloads: 544501614 | Hello! Modify this message while the program is running.
OK reloads: 1869376609 | Total reloads: 544501614 | Hello! Modify this message while the program is running.

But it seems that the reload counter has some problem.

@kbkpbot
Copy link
Contributor Author

kbkpbot commented Jan 3, 2025

Maybe we can do this access the g_live_info symbol in main:

  1. VV_EXPORTED_SYMBOL void* g_live_info = NULL;
  2. In the shared dll, use GetProcessAddress get the g_live_info address.

@kbkpbot
Copy link
Contributor Author

kbkpbot commented Jan 3, 2025

As under Windows, it is difficult that make the .dll access the caller's symbol, I suggest that move the g_live_info structure into the .dll.

As the .dll will be compiled and reloaded, it is not suitable put the g_live_info struct in the .dll.

vlib/v/live/common.c.v Outdated Show resolved Hide resolved
@kbkpbot
Copy link
Contributor Author

kbkpbot commented Jan 3, 2025

	if C.g_live_info == 0 {
		// When the current program is not compiled with -live, simply
		// return a new empty struct LiveReloadInfo in order to prevent
		// crashes. In this case, the background reloader thread is not
		// started, and the structure LiveReloadInfo will not get updated.
		// All its fields will be 0, but still safe to access.
		unsafe { C.g_live_info = int(&LiveReloadInfo{}) }
	}
	return unsafe { &LiveReloadInfo(C.g_live_info) }

This will generated a simplify code:

	if (g_live_info == 0) {
		{ // Unsafe block
			g_live_info =((int)(((v__live__LiveReloadInfo*)memdup(...}, sizeof(v__live__LiveReloadInfo)));
		}
	}
	return ((v__live__LiveReloadInfo*)(g_live_info));

vlib/v/live/common.c.v Outdated Show resolved Hide resolved
@phcreery
Copy link
Contributor

phcreery commented Jan 3, 2025

I see this overlaps most of everything from an old PR except https://github.com/vlang/v/pull/19621/files#diff-c8aafc93f5a132c1afb63f4c56a73731d8368f0076262fc9fe26a209661690b6

@spytheman
Copy link
Member

There are some similarities, but this PR fixes a much deeper problem, and in a more maintainable way.

@spytheman spytheman merged commit f821c65 into vlang:master Jan 3, 2025
72 checks passed
@kbkpbot kbkpbot deleted the fix-windows-hot-reload branch January 3, 2025 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants