Re: Problem with Integrated Vim Editor on Win 10
I dug into this some more, and as surprising as it is, I believe the release of Git 2.8.0 is just busted. I had an installer for 2.7.0 lying around, so after uninstalling 2.8.0 and re-installing 2.7.0, everything works fine. I'm not terribly active in the Git community so I don't know what the procedure is for things like this, but this seems like a fairly serious regression. Suggestions on how to proceed? On Wed, Mar 30, 2016 at 5:07 PM, Zachary Turner <ztur...@google.com> wrote: > Hi, just recently I installed the latest build of Windows 10 of my > machine. This is my second Win10 machine. On the other I am using git > 2.7.0.windows.1 and everything is working just fine. > > On the second machine I am using git 2.8.0.windows.1 and vim does not > work. I sent a bug report to b...@vim.org, but frankly I don't know whose > bug this is, so I'm including it here as well. > > The problem is that vim is just a black screen when git launches it. If I > mash enough keys eventually I see something that resembles vim output at > the bottom, but I can't actually use it. > > I tried going into program files\git\usr\bin and just running vim.exe. > Again, black screen. If I press enter about 10 times I can see the > introduction screen. Then if I press : about 10-20 times it will go into > command mode and a single : appears. after pressing a few more keys all > the rest of the :s appear. Basically, everything is completely unusable. > > I tried downloading vim 7.4 from www.vim.org, and low and behold, it > works. For now I've replaced the copy of vim.exe that ships with git with > the copy from www.vim.org. But this leaves me nervous that something is > seriously wrong. > > Has anyone seen anything like this before, or have any ideas how I might > better diagnose this? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Problem with Integrated Vim Editor on Win 10
Hi, just recently I installed the latest build of Windows 10 of my machine. This is my second Win10 machine. On the other I am using git 2.7.0.windows.1 and everything is working just fine. On the second machine I am using git 2.8.0.windows.1 and vim does not work. I sent a bug report to b...@vim.org, but frankly I don't know whose bug this is, so I'm including it here as well. The problem is that vim is just a black screen when git launches it. If I mash enough keys eventually I see something that resembles vim output at the bottom, but I can't actually use it. I tried going into program files\git\usr\bin and just running vim.exe. Again, black screen. If I press enter about 10 times I can see the introduction screen. Then if I press : about 10-20 times it will go into command mode and a single : appears. after pressing a few more keys all the rest of the :s appear. Basically, everything is completely unusable. I tried downloading vim 7.4 from www.vim.org, and low and behold, it works. For now I've replaced the copy of vim.exe that ships with git with the copy from www.vim.org. But this leaves me nervous that something is seriously wrong. Has anyone seen anything like this before, or have any ideas how I might better diagnose this? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
It shouldn't be hard for us to run some tests with this patch applied. Will report back in a day or two. On Tue, Feb 18, 2014 at 9:55 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Sat, Feb 15, 2014 at 8:15 AM, Zachary Turner ztur...@chromium.org wrote: ... 2) Use TLS as you suggest and have one fd per pack thread. Probably the most complicated code change (at least for me, being a first-time contributor) It's not so complicated. I suggested a patch [1] before (surprise!). ... [1] http://article.gmane.org/gmane.comp.version-control.git/196042 That message is at the tail end of the discussion. I wonder why nothing came out of it back then. While I do not see anything glaringly wrong with the change from a quick glance over it, it would be nice to hear how well it performs on their platform from Windows folks. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
(Gah, sorry if you're receiving multiple emails to your personal
addresses, I need to get used to manually setting Plain-text mode
every time I send a message).
For the mixed read, we wouldn't be looking for another caller of
pread() (since it doesn't care what the file pointer is), but instead
a caller of read() or lseek() (since those do depend on the current
file pointer). In index-pack.c, I see two possible culprits:
1) A call to xread() from inside fill()
2) A call to lseek in parse_pack_objects()
Do you think these could be related? If so, maybe that opens up some
other solutions?
BTW, the version you posted isn't thread safe. Suppose thread A and
thread B execute this function at the same time. A executes through
the ReadFile(), but does not yet reset the second lseek64. B then
executes the first lseek64(), storing off the modified file pointer.
Then A finishes, then B finishes. At the end, the file pointer is
still modified.
On Fri, Feb 14, 2014 at 11:15 AM, Zachary Turner ztur...@chromium.org wrote:
For the mixed read, we wouldn't be looking for another caller of pread()
(since it doesn't care what the file pointer is), but instead a caller of
read() or lseek(). In index-pack.c, I see two possible culprits:
1) A call to xread() from inside fill()
2) A call to lseek in parse_pack_objects()
Do you think these could be related? If so, maybe that opens up some other
solutions?
BTW, the version you posted isn't thread safe. Suppose thread A and thread
B execute this function at the same time. A executes through the
ReadFile(), but does not yet reset the second lseek64. B then executes the
first lseek64(), storing off the modified file pointer. Then A finishes,
then B finishes. At the end, the file pointer is still modified.
On Fri, Feb 14, 2014 at 11:04 AM, Karsten Blees karsten.bl...@gmail.com
wrote:
Am 14.02.2014 00:09, schrieb Zachary Turner:
To elaborate a little bit more, you can verify with a sample program
that ReadFile with OVERLAPPED does in fact modify the HANDLE's file
position. The documentation doesn't actually state one way or
another. My original attempt at a patch didn't have the ReOpenFile,
and we experienced regular read corruption. We scratched our heads
over it for a bit, and then hypothesized that someone must be mixing
read styles, which led to this ReOpenFile workaround, which
incidentally also solved the corruption problems. We wrote a similar
sample program to verify that when using ReOpenHandle, and changing
the file pointer of the duplicated handle, that the file pointer of
the original handle is not modified.
We did not actually try to identify the source of the mixed read
styles, but it seems like the only possible explanation.
On Thu, Feb 13, 2014 at 2:53 PM, Stefan Zager sza...@google.com wrote:
On Thu, Feb 13, 2014 at 2:51 PM, Karsten Blees
karsten.bl...@gmail.com wrote:
Am 13.02.2014 19:38, schrieb Zachary Turner:
The only reason ReOpenFile is necessary at
all is because some code somewhere is mixing read-styles against the
same
fd.
I don't understand...ReadFile with OVERLAPPED parameter doesn't modify
the HANDLE's file position, so you should be able to mix read()/pread()
however you like (as long as read() is only called from one thread).
That is, apparently, a bald-faced lie in the ReadFile API doc. First
implementation didn't use ReOpenFile, and it crashed all over the
place. ReOpenFile fixed it.
Stefan
Damn...you're right, multi-threaded git-index-pack works fine, but some
tests fail badly. Mixed reads would have to be from git_mmap, which is the
only other caller of pread().
A simple alternative to ReOpenHandle is to reset the file pointer to its
original position, as in compat/pread.c::git_pread. Thus single-theaded code
can mix read()/pread() at will, but multi-threaded code has to use pread()
exclusively (which is usually the case anyway). A main thread using read()
and background threads using pread() (which is technically allowed by POSIX)
will fail with this solution.
This version passes the test suite on msysgit:
8
ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
{
DWORD bytes_read;
OVERLAPPED overlapped;
off64_t current;
memset(overlapped, 0, sizeof(overlapped));
overlapped.Offset = (DWORD) offset;
overlapped.OffsetHigh = (DWORD) (offset 32);
current = lseek64(fd, 0, SEEK_CUR);
if (!ReadFile((HANDLE)_get_osfhandle(fd), buf, count, bytes_read,
overlapped)) {
errno = err_win_to_posix(GetLastError());
return -1;
}
lseek64(fd, current, SEEK_SET);
return (ssize_t) bytes_read;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
Even if we make that change to use TLS for this case, the implementation of pread() will still change in such a way that the semantics of pread() are different on Windows. Is that ok? Just to summarize, here's the viable approaches I've seen discussed so far: 1) Use _WINVER at compile time to select either a thread-safe or non-thread-safe implementation of pread. This is the easiest possible code change, but would necessitate 2 binary distributions of git for windows. 2) Use TLS as you suggest and have one fd per pack thread. Probably the most complicated code change (at least for me, being a first-time contributor) 3) Use Karsten's suggested implementation from earlier in the thread. Seems to work, but it's a little confusing from a readability standpoint since the implementation is not-thread safe except in this specific usage context. On Fri, Feb 14, 2014 at 4:56 PM, Duy Nguyen pclo...@gmail.com wrote: On Sat, Feb 15, 2014 at 7:50 AM, Stefan Zager sza...@google.com wrote: On Fri, Feb 14, 2014 at 4:45 PM, Duy Nguyen pclo...@gmail.com wrote: On Sat, Feb 15, 2014 at 2:16 AM, Zachary Turner ztur...@chromium.org wrote: (Gah, sorry if you're receiving multiple emails to your personal addresses, I need to get used to manually setting Plain-text mode every time I send a message). For the mixed read, we wouldn't be looking for another caller of pread() (since it doesn't care what the file pointer is), but instead a caller of read() or lseek() (since those do depend on the current file pointer). In index-pack.c, I see two possible culprits: 1) A call to xread() from inside fill() 2) A call to lseek in parse_pack_objects() Do you think these could be related? If so, maybe that opens up some other solutions? For index-pack alone, what's wrong with open one file handle per thread? Nothing wrong with that, except that it would mean either using thread-local storage (which the code doesn't currently use); or plumbing pack_fd through the call stack, which doesn't sound very fun. Current code does use thread-local storage (struct thread_local and get_thread_data). Adding a new file handle when NO_THREAD_SAFE_PREAD is defined is simpler imo. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
Karsten Blees karsten.blees at gmail.com writes: Am 12.02.2014 19:37, schrieb Erik Faye-Lund: On Wed, Feb 12, 2014 at 7:34 PM, Stefan Zager szager at google.com wrote: On Wed, Feb 12, 2014 at 10:27 AM, Erik Faye-Lund kusmabite at gmail.com wrote: On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager szager at google.com wrote: I don't want to steal the thunder of my coworker, who wrote the implementation. He plans to submit it upstream soon-ish. It relies on using the lpOverlapped argument to ReadFile(), with some additional tomfoolery to make sure that the implicit position pointer for the file descriptor doesn't get modified. Is the code available somewhere? I'm especially interested in the additional tomfoolery to make sure that the implicit position pointer for the file descriptor doesn't get modified-part, as this was what I ended up butting my head into when trying to do this myself. https://chromium-review.googlesource.com/#/c/186104/ ReOpenFile, that's fantastic. Thanks a lot! ...but should be loaded dynamically via GetProcAddress, or are we ready to drop XP support? Original patch author here. In trying to prepare this patch to use GetProcAddress to load dynamically, I've run into a bit of a snag. NO_THREAD_SAFE_PREAD is a compile-time flag, which will be incompatible with any attempt to make this a runtime decision a la LoadLibrary / GetProcAddress. On XP, we would need to fallback to the single-threaded path, and on Vista+ we would use the thread-able path, and obviously this decision could not be made until runtime. If MinGW were the only configuration using NO_THREAD_SAFE_PREAD, I would just remove it entirely, but it appears Cygwin configuration uses it also. Suggestions? One possibility is to disallow (by convention, perhaps), the use of pread() and read() against the same fd. The only reason ReOpenFile is necessary at all is because some code somewhere is mixing read-styles against the same fd. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
To elaborate a little bit more, you can verify with a sample program that ReadFile with OVERLAPPED does in fact modify the HANDLE's file position. The documentation doesn't actually state one way or another. My original attempt at a patch didn't have the ReOpenFile, and we experienced regular read corruption. We scratched our heads over it for a bit, and then hypothesized that someone must be mixing read styles, which led to this ReOpenFile workaround, which incidentally also solved the corruption problems. We wrote a similar sample program to verify that when using ReOpenHandle, and changing the file pointer of the duplicated handle, that the file pointer of the original handle is not modified. We did not actually try to identify the source of the mixed read styles, but it seems like the only possible explanation. On Thu, Feb 13, 2014 at 2:53 PM, Stefan Zager sza...@google.com wrote: On Thu, Feb 13, 2014 at 2:51 PM, Karsten Blees karsten.bl...@gmail.com wrote: Am 13.02.2014 19:38, schrieb Zachary Turner: The only reason ReOpenFile is necessary at all is because some code somewhere is mixing read-styles against the same fd. I don't understand...ReadFile with OVERLAPPED parameter doesn't modify the HANDLE's file position, so you should be able to mix read()/pread() however you like (as long as read() is only called from one thread). That is, apparently, a bald-faced lie in the ReadFile API doc. First implementation didn't use ReOpenFile, and it crashed all over the place. ReOpenFile fixed it. Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
