move writing to REPL history file to behind a PID lock

[KristofferC]

Should fix #37015

[KristofferC]

Looks like PID files are not getting deleted on windows.

ERROR: The working directory is dirty.
Output of git status:
On branch refs/pull/45450/merge
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	base/$16NCnul.pid.deleted
	base/$1ARWnul.pid.deleted
	base/$3OHYnul.pid.deleted
	base/$5I78nul.pid.deleted
	base/$5IRKnul.pid.deleted
	base/$5MBOnul.pid.deleted
	base/$5UBGnul.pid.deleted
	base/$74L2nul.pid.deleted
	base/$9AB4nul.pid.deleted
	base/$9AJCnul.pid.deleted
	base/$BS1Anul.pid.deleted
	base/$D2NGnul.pid.deleted
	base/$H2RWnul.pid.deleted
	base/$JC5Ynul.pid.deleted
	base/$LAB4nul.pid.deleted
	base/$PUV4nul.pid.deleted
	base/$RWHAnul.pid.deleted
	base/$XANOnul.pid.deleted
	base/$XI30nul.pid.deleted
	base/$XY3Knul.pid.deleted

Also:

┌ Error: Error in the keymap
│   exception =
│    IOError: open("/dev/null.pid", 194, 292): permission denied (EACCES)
│    Stacktrace:

[KristofferC]

I don't get these .deleted files building it locally with cygwin.

[vtjnash]

This seems to be a bug in our rename function. Julia has no way to do such a fundamental operation as just renaming a file, and always does other junk with it. So when someone make a pidfile on the illegal file name nul.pid, we make a copy of the empty nul file instead.

[KristofferC]

@vtjnash, does this look ok now?

[vtjnash]

SGTM, though looks mildly concerning that the new test segfaulted the windows worker and causes temp_cleanup_purge to crash

[KristofferC]

Yeah, that's not great...

       From worker 8:	Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
      From worker 8:	Exception: EXCEPTION_ACCESS_VIOLATION at 0x6d37d1ba -- ijl_load_dynamic_library at /cygdrive/c/buildbot/worker/package_win32/build/src\dlload.c:230
      From worker 8:	in expression starting at C:\buildbot\worker-tabularasa\tester_win32\build\share\julia\stdlib\v1.9\REPL\test\repl.jl:1011
      From worker 8:	ijl_load_dynamic_library at /cygdrive/c/buildbot/worker/package_win32/build/src\dlload.c:230
      From worker 8:	jl_get_library_ at /cygdrive/c/buildbot/worker/package_win32/build/src\runtime_ccall.cpp:48
      From worker 8:	jl_get_library_ at /cygdrive/c/buildbot/worker/package_win32/build/src\runtime_ccall.cpp:39 [inlined]
      From worker 8:	ijl_load_and_lookup at /cygdrive/c/buildbot/worker/package_win32/build/src\runtime_ccall.cpp:61
      From worker 8:	jlplt_gethostname_59271.clone_1 at C:\buildbot\worker-tabularasa\tester_win32\build\lib\julia\sys.dll (unknown line)
      From worker 8:	write_pidfile at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\FileWatching\src\pidfile.jl:113
      From worker 8:	mkpidlock#1 at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\FileWatching\src\pidfile.jl:59
      From worker 8:	mkpidlock##kw at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\FileWatching\src\pidfile.jl:51 [inlined]
      From worker 8:	#mkpidlock#7 at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\FileWatching\src\pidfile.jl:80
      From worker 8:	mkpidlock##kw at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\FileWatching\src\pidfile.jl:79 [inlined]
      From worker 8:	#mkpidlock#6 at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\FileWatching\src\pidfile.jl:77 [inlined]
      From worker 8:	mkpidlock##kw at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\FileWatching\src\pidfile.jl:77 [inlined]
      From worker 8:	add_history at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.9\REPL\src\REPL.jl:629

[KristofferC]

@vtjnash, any idea why it would crash in gethostname on win32?

[vtjnash]

No, that is very strange. It is supposed to be stack memory there, or the constant string "ws2_32", depending on where it is failing at that line. Or the stack is smashed by that code there (missing a \0 perhaps), and the stacktrace is bogus?

[mbauman]

Is the situation here better now that we have similar PID locking for Manifest in Pkg? Cf JuliaLang/Pkg.jl#2793

[mbauman]

I guess let's just try CI again with a rebase

[KristofferC]

I don't think Pkg is relevant here and I am not sure the pid file code has changed much but still interesting to rebase this.

[paciorek]

Is there any energy to come back to this issue and put a fix in place? I'm having frequent REPL history file contamination when having multiple Julia sessions on a Linux cluster with a shared home directory.

[StefanKarpinski]

Yes, I think that we should go ahead with this.

[mbauman]

Looks like the tests now pass on Windows!? The only failure was an OOM on 32-bit linux, but that was back in February.