baudehlo / node-fs-ext

Extras missing from node's fs module
MIT License
101 stars 45 forks source link

Module not working on Windows #30

Closed bajtos closed 9 years ago

bajtos commented 10 years ago

The HEAD version does not work on Windows 8.1 x64.

$ node tests\test-fs-flock.js
FAILURE: LOCK_EX is not defined correctly
  LOCK_EX    undefined    "undefined"
FAILURE: LOCK_NB is not defined correctly
  LOCK_NB    undefined    "undefined"
FAILURE: LOCK_SH is not defined correctly
  LOCK_SH    undefined    "undefined"
FAILURE: LOCK_UN is not defined correctly
  LOCK_UN    undefined    "undefined"
FAILURE: expect_errno: flockSync(): expected error EBADF, got another error
   ARGS:  { '0': 'flockSync',
  '1': -9,
  '2': [TypeError: Bad argument],
  '3': 'EBADF' }
FAILURE: expect_errno: flockSync(): expected error EBADF, got another error
   ARGS:  { '0': 'flockSync',
  '1': 98765,
  '2': [TypeError: Bad argument],
  '3': 'EBADF' }
FAILURE: expect_ok: flockSync(): returned error
   ARGS:  { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] }
FAILURE: expect_ok: flockSync(): returned error
   ARGS:  { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] }
FAILURE: expect_ok: flockSync(): returned error
   ARGS:  { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] }
FAILURE: expect_ok: flockSync(): returned error
   ARGS:  { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] }
FAILURE: expect_ok: flockSync(): returned error
   ARGS:  { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] }

When used in Sinopia, it crashes the whole process. Release build of node v0.10.26 crashes with StackOverflow.

The debug build of node v0.10.26 reports

Unhandled exception at 0x6D5633D1 (fs-ext.node) in node.exe: An invalid parameter was passed to a function that considers invalid parameters fatal.

Stack trace:
    fs-ext.node!_invalid_parameter_noinfo() Line 96 C++
    fs-ext.node!_get_osfhandle(int fh) Line 306 C
    fs-ext.node!_win32_flock(int fd, int oper) Line 196 C++
>   fs-ext.node!EIO_Flock(uv_work_s * req) Line 247 C++
baudehlo commented 10 years ago

Unfortunately I don't have Windows boxes to test/develop on. I'm relying on the open source community to work on the Windows compatibility.

On Mon, Mar 24, 2014 at 10:13 AM, Miroslav Bajtoš notifications@github.comwrote:

The HEAD version does not work on Windows 8.1 x64.

$ node tests\test-fs-flock.js FAILURE: LOCK_EX is not defined correctly LOCK_EX undefined "undefined" FAILURE: LOCK_NB is not defined correctly LOCK_NB undefined "undefined" FAILURE: LOCK_SH is not defined correctly LOCK_SH undefined "undefined" FAILURE: LOCK_UN is not defined correctly LOCK_UN undefined "undefined" FAILURE: expect_errno: flockSync(): expected error EBADF, got another error ARGS: { '0': 'flockSync', '1': -9, '2': [TypeError: Bad argument], '3': 'EBADF' } FAILURE: expect_errno: flockSync(): expected error EBADF, got another error ARGS: { '0': 'flockSync', '1': 98765, '2': [TypeError: Bad argument], '3': 'EBADF' } FAILURE: expect_ok: flockSync(): returned error ARGS: { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] } FAILURE: expect_ok: flockSync(): returned error ARGS: { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] } FAILURE: expect_ok: flockSync(): returned error ARGS: { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] } FAILURE: expect_ok: flockSync(): returned error ARGS: { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] } FAILURE: expect_ok: flockSync(): returned error ARGS: { '0': 'flockSync', '1': 3, '2': [TypeError: Bad argument] }

When used in Sinopia https://github.com/rlidwka/sinopia, it crashes the whole process. Release build of node v0.10.26 crashes with StackOverflow.

The debug build of node v0.10.26 reports

Unhandled exception at 0x6D5633D1 (fs-ext.node) in node.exe: An invalid parameter was passed to a function that considers invalid parameters fatal.

Stack trace: fs-ext.node!_invalid_parameter_noinfo() Line 96 C++ fs-ext.node!_get_osfhandle(int fh) Line 306 C fs-ext.node!_win32_flock(int fd, int oper) Line 196 C++

fs-ext.node!EIO_Flock(uv_work_s * req) Line 247 C++

— Reply to this email directly or view it on GitHubhttps://github.com/baudehlo/node-fs-ext/issues/30 .

bajtos commented 10 years ago

Unfortunately I don't have Windows boxes to test/develop on. I'm relying on the open source community to work on the Windows compatibility.

Sure. My primary motivation was to record the problem, so that other people can find this issue when troubleshooting Sinopia or fs-ext on Windows.

@piscisaureus and me spent some time tracking down the issue, but were not able to pin down the root cause yet. One possible reason may be duplication of the fd->osfhandle table between Node core and the native module. I'll post an update if we find anything more.

winnetou1357 commented 10 years ago

The failure is that the names LOCK_EX, LOCK_SH, LOCK_NB and LOCK_UN are accidentally defined as const ints, while NODE_DEFINE_CONSTANT calls are guarded with #ifdefs for these names. See fs-ext.cc:76 and fs-ext.cc:455 Now the code compiles gracefully, but any call to flock result in a TypeError with the message "Bad argument", because stringToFlockFlags converts anything to undefined.

@baudehlo I would willingly make a pull request for it but I'm not familiar with git and github anyway. I plan to learn it soon, but until that, feel free to fix this issue.

baudehlo commented 10 years ago

I don't have windows to test against so a pull request would be preferable.

On May 15, 2014, at 8:32 AM, winnetou1357 notifications@github.com wrote:

The failure is that the names LOCK_EX, LOCK_SH, LOCK_NB and LOCK_UN are accidentally defined as const ints, while NODE_DEFINE_CONSTANT calls are guarded with #ifdefs for these names. See fs-ext.cc:76 and fs-ext.cc:455 Now the code compiles gracefully, but any call to flock result in a TypeError with the message "Bad argument", because stringToFlockFlags converts anything to undefined.

@baudehlo I would willingly make a pull request for it but I'm not familiar with git and github anyway. I plan to learn it soon, but until that, feel free to fix this issue.

— Reply to this email directly or view it on GitHub.

bajtos commented 10 years ago

I have submitted a pull request fixing the constant vs. macro problem.

Now the flock test node tests\test-fs-flock.js silently crashes, which is the real problem and much more difficult to solve. As I said in a previous comment, it's possible the duplication of the fd->osfhandle table between Node core and the native module is broken on Windows. The issue is hard to debug, as the Node runtime tends to crash on a different thing when the test is run in a native VisualStudio debugger.

bajtos commented 10 years ago

@baudehlo

I don't have windows to test against so a pull request would be preferable.

FWIW, you can download a Windows image for VirtualBox (or other VM host) for free at http://www.modern.ie/en-us/virtualization-tools#downloads

tracker1 commented 9 years ago

@baudehlo to add to what @bajtos pointed to...

Visual Studio Express 2013 Using VS2013 Express with Platform SDK Platform SDK (search, match the windows version)

If I had a better understanding of C/C++, I'd love to help with this... since this level of locking (and the need for range locking) are kind of important to me... as it is, I'm shimming the functionality I need via edge which works, but is not as lean as I'd really like it to be.

baudehlo commented 9 years ago

Sadly those don't work on Macs :) And my main dev machine at home isn't powerful enough to run VMs.

On Thu, Oct 23, 2014 at 2:59 PM, Michael J. Ryan notifications@github.com wrote:

@baudehlo https://github.com/baudehlo to add to what @bajtos https://github.com/bajtos pointed to...

Visual Studio Express 2013 http://www.visualstudio.com/downloads/download-visual-studio-vs#d-express-web Using VS2013 Express with Platform SDK http://msdn.microsoft.com/en-US/library/ms235626(v=vs.80).aspx Platform SDK (search, match the windows version) http://www.microsoft.com/en-us/search/result.aspx?q=platform%20sdk%20windows%207

If I had a better understanding of C/C++, I'd love to help with this... since this level of locking (and the need for range locking) are kind of important to me... as it is, I'm shimming the functionality I need via edge https://www.npmjs.org/package/edge which works, but is not as lean as I'd really like it to be.

— Reply to this email directly or view it on GitHub https://github.com/baudehlo/node-fs-ext/issues/30#issuecomment-60291049.

tracker1 commented 9 years ago

VirtualBox doesn't work on Mac?

baudehlo commented 9 years ago

Yes it does, but I only have 2G of ram at home. I have plenty on my dev machine at work, but no time (or reason for the company to need it) to do the work there.

On Fri, Oct 24, 2014 at 12:26 AM, Michael J. Ryan notifications@github.com wrote:

VirtualBox doesn't work on Mac?

— Reply to this email directly or view it on GitHub https://github.com/baudehlo/node-fs-ext/issues/30#issuecomment-60343364.

simonhoss commented 9 years ago

Hi,

I found the problem. The problem is that fs-ext tries to open an invalid file descriptor (EBADF). So I made a fix that node does not crash, when this happens. But I cannot resolve why the handle is invalid. Any ideas?

Here is my test code:

var fsExt = require("./fs-ext");
var fs = require("fs");
fs.open("D:/test/test.txt", "r", function(err, fd) { 
  fsExt.flock(fd, 6, function(test) { console.log(test); });
});
bajtos commented 9 years ago

@simonhoss See my comment above. The problem seems to be caused by the way how Node.js works with Windows file handles, where the handles seem to be unavailable to native addons. As I mentioned earlier, I was debugging this with Bert, who wrote most of the Windows-specific code in Node.js core, and even he could not track down the source of the problem in a reasonably short time.

simonhoss commented 9 years ago

Ok. So then I make a pull request that node does not crash and brings a EBADF error message. #36

baudehlo commented 9 years ago

I'm closing this issue now due to the merged pull requests. Please re-open if it's still an issue.

bajtos commented 9 years ago

I don't think #36 fixed the problem, see my comment: https://github.com/baudehlo/node-fs-ext/pull/36#issuecomment-76450010.

However, since there was nobody else complaining about the Windows support in the last three months or so, I think it's ok to keep this issue closed.