Additional fixes. #39
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Unfortunately there's no other way to suppress them optionally that I can find, and they were the only warnings in our build.
Import paths can be in the format:
relative/to/script.js
/absolute/path/to/script.js
~/relative/to/home/script.js
BohemianCoding/Sketch#3319 Script import: Add support for absolute and ~ paths
Also guards against multiple imports of the same URL, and therefore circular imports Ref BohemianCoding/Sketch#3319
BohemianCoding/Sketch#3319 Add support for nested @import statements
Fix a bug which ignored first line of @import-ed scripts
The theory here is that JS references to objects may be in the ffi arguments as raw pointers, so it's a good idea to protect everything. May be spurious...
…get created but aren't referenced from JS anywhere at the point the struct is made, so I suspect that they might be destroyed again immediately.
We will "leak" any object in the map until the JS reference is garbage collected, or until the end of the script session - which is not ideal, but better than crashing.
…and ensure everything gets disposed of.
# Conflicts: # Cocoa Script.xcodeproj/project.pbxproj # src/framework/mocha/MochaRuntime.m # src/framework/mocha/Objects/MOBox.m
Make sure that boxes get dissassociated and clean out the JS object's private pointer, before they are dealloced.
…tom class instead of a dictionary.
|
(if we ever conclude that this code is good, I can make more of an attempt to factor it out into some more manageable PRs - for now this is mostly just FYI) |
samdeane
force-pushed
the
feature/7496-latest-sketch-beta-crashes
branch
from
4f65077 to
363a1eb
Compare
|
Thanks- I'll take a look at this when I get a chance. I'd like to merge in all your changes as well- but there's some stuff that I need to sort through and figure out what's breaking Acorn. |
|
Sure - no hurry, and just yell if you need a hand disentangling our stuff :) |
|
Oops - this branch got deleted on our end, so the PR went away. I'll make another. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an evolution of the code in PR #34. We've had some crashes reported for Sketch using that code, so I've refined it further.
I still don't think I've got all the crashes out, so I will probably have to reinstate the JSValueProtect hack in the meantime.
Debugging this code is quite interesting though.
I've downgraded a couple of the assertions to just be debug() statements, and I get the "box should have been disassociated" one firing occasionally, seemingly when things are being cleaned up.
It looks like it's firing for something that was passed as a parameter in a function call, though I'm not 100% certain.