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

Fix none resolution #66

Closed
wants to merge 1 commit into from
Closed

Conversation

medikoo
Copy link
Contributor

@medikoo medikoo commented Jul 28, 2015

I have configuration where I call st multiple times, and following setup lead to errors:

st({ path: '/some/public/path', cache: { fd: false } });
st({ path: '/some/public/path2', cache: false });

Problem is that with first call, internally reusable none object is modified:
https://github.com/isaacs/st/blob/master/st.js#L136
https://github.com/isaacs/st/blob/master/st.js#L157

With second st intialization, dispose goes to all other caches, and that at some point crashes sever:

fs.js:465
  binding.close(fd, req);
          ^
TypeError: Bad argument
    at TypeError (native)
    at Object.fs.close (fs.js:465:11)
    at Object.close (/Users/medikoo/Projects/eregistrations/lomas/node_modules/graceful-fs/graceful-fs.js:37:19)
    at Object.cleanupFd (/Users/medikoo/Projects/eregistrations/lomas/node_modules/st/node_modules/fd/index.js:21:12)
    at Object.<anonymous> (/Users/medikoo/Projects/eregistrations/lomas/node_modules/st/node_modules/fd/index.js:35:21)
    at process._tickCallback (node.js:355:11)

It's caused by stats objects from stat cache being provided to dispose of previous setup fd cache.

I didn't provide test for it, as I'm not that familiar with current test setup, if it's something straightforward please guide and I'll add them.

`none` was provided in direct form and modified afterwards,
so next calls of st, modified `none` was reused
@rvagg
Copy link
Collaborator

rvagg commented Jul 28, 2015

oh, good catch, it took me a bit to follow your logic of it being modified but I see the attached dispose and load functions now

@rvagg
Copy link
Collaborator

rvagg commented Jul 28, 2015

landed as 16c5fe2, published as 0.5.5, cheers

@rvagg rvagg closed this Jul 28, 2015
@medikoo
Copy link
Contributor Author

medikoo commented Jul 28, 2015

Thank you!

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.

2 participants