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

Backporting Async Wrap changes #7048

Closed
wants to merge 18 commits into from
Closed

Backporting Async Wrap changes #7048

wants to merge 18 commits into from

Commits on Jun 7, 2016

  1. src: fix MakeCallback error handling

    Implementations of error handling between node::MakeCallback() and
    AsyncWrap::MakeCallback() do not return at the same point. Make both
    executions work the same by moving the early return if there's a caught
    exception just after the AsyncWrap post callback. Since the domain's
    call stack is cleared on a caught exception there is no reason to call
    its exit() callback.
    
    Remove the SetVerbose() statement in the AsyncWrap pre/post callback
    calls since it does not affect the callback call.
    
    PR-URL: #4507
    Reviewed-By: Fedor Indutny <fedor@indutny.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    b5319a5 View commit details
    Browse the repository at this point in the history
  2. src: add AsyncCallbackScope

    Add a scope that will allow MakeCallback to know whether or not it's
    currently running. This will prevent nextTickQueue and the
    MicrotaskQueue from being processed recursively. It is also required to
    wrap the bootloading stage since it doesn't run within a MakeCallback.
    
    Ref: nodejs/node-v0.x-archive#9245
    PR-URL: #4507
    Reviewed-By: Fedor Indutny <fedor@indutny.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    19dfb2a View commit details
    Browse the repository at this point in the history
  3. src: remove unused of TickInfo::last_threw()

    Environment::TickInfo::last_threw() is no longer in use.
    
    Also pass Isolate to few methods and fix whitespace alignment.
    
    PR-URL: #4507
    Reviewed-By: Fedor Indutny <fedor@indutny.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    7d369f1 View commit details
    Browse the repository at this point in the history
  4. src: remove unused TickInfo::in_tick()

    PR-URL: #4507
    Reviewed-By: Fedor Indutny <fedor@indutny.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    0da31a1 View commit details
    Browse the repository at this point in the history
  5. src: remove TryCatch in MakeCallback

    After attempting to use ReThrow() and Reset() there were cases where
    firing the domain's error handlers was not happening. Or in some cases
    reentering MakeCallback would still cause the domain enter callback to
    abort (because the error had not been Reset yet).
    
    In order for the script to properly stop execution when a subsequent
    call to MakeCallback throws it must not be located within a TryCatch.
    
    PR-URL: #4507
    Reviewed-By: Fedor Indutny <fedor@indutny.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    a6a7382 View commit details
    Browse the repository at this point in the history
  6. test: add addons test for MakeCallback

    Make sure that calling MakeCallback multiple times within the same stack
    does not allow the nextTickQueue or MicrotaskQueue to be processed in
    any more than the first MakeCallback call.
    
    Check that domains enter/exit poperly with multiple MakeCallback calls
    and that errors are handled as expected
    
    PR-URL: #4507
    Reviewed-By: Fedor Indutny <fedor@indutny.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    1adb057 View commit details
    Browse the repository at this point in the history
  7. src: remove unnecessary check

    The value's type is unsigned so it will always be >= 0.
    
    PR-URL: #5233
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    mscdex authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    a321e6d View commit details
    Browse the repository at this point in the history
  8. async_wrap: add uid to all asyncWrap hooks

    By doing this users can use a Map object for storing information
    instead of modifying the handle object.
    
    PR-URL: #4600
    Reviewed-By: Trevor Norris <trev.norris@gmail.com>
    Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
    AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    331b85c View commit details
    Browse the repository at this point in the history
  9. async_wrap: make uid the first argument in init

    All other hooks have uid as the first argument, this makes it concistent
    for all hooks.
    
    PR-URL: #4600
    Reviewed-By: Trevor Norris <trev.norris@gmail.com>
    Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
    AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    1c9cd4a View commit details
    Browse the repository at this point in the history
  10. async_wrap: add parent uid to init hook

    When the parent uid is required it is not necessary to store the uid in
    the parent handle object.
    
    PR-URL: #4600
    Reviewed-By: Trevor Norris <trev.norris@gmail.com>
    Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
    AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    051103d View commit details
    Browse the repository at this point in the history
  11. http_parser: use MakeCallback

    Make `HTTPParser` an instance of `AsyncWrap` and make it use
    `MakeCallback`. This means that async wrap hooks will be called on
    consumed TCP sockets as well as on non-consumed ones.
    
    Additional uses of `AsyncCallbackScope` are necessary to prevent
    improper state from progressing that triggers failure in the
    test-http-pipeline-flood.js test. Optimally this wouldn't be necessary,
    but for the time being it's the most sure way to allow operations to
    proceed as they have.
    
    Fix: #4416
    PR-URL: #5419
    Reviewed-By: Fedor Indutny <fedor@indutny.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    8f4438d View commit details
    Browse the repository at this point in the history
  12. src,http: fix uncaughtException miss in http

    In AsyncWrap::MakeCallback always return empty handle if there is an
    error. In the future this should change to return a v8::MaybeLocal, but
    that major change will have to wait for v6.x, and these changes are
    meant to be backported to v4.x.
    
    The HTTParser call to AsyncWrap::MakeCallback failed because it expected
    a thrown call to return an empty handle.
    
    In node::MakeCallback return an empty handle if the call is
    in_makecallback(), otherwise return v8::Undefined() as usual to preserve
    backwards compatibility.
    
    Fixes: #5555
    PR-URL: #5591
    Reviewed-By: Julien Gilli <jgilli@nodejs.org>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    71d7862 View commit details
    Browse the repository at this point in the history
  13. src,http_parser: remove KickNextTick call

    Now that HTTPParser uses MakeCallback it is unnecessary to manually
    process the nextTickQueue.
    
    The KickNextTick function is now no longer needed so code has moved back
    to node::MakeCallback to simplify implementation.
    
    Include minor cleanup moving Environment::tick_info() call below the
    early return to save an operation.
    
    PR-URL: #5756
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    d94eec1 View commit details
    Browse the repository at this point in the history
  14. src: reword command and add ternary

    Make comment clear that Undefined() is returned for legacy
    compatibility. This will change in the future as a semver-major change,
    but to be able to port this to previous releases it needs to stay as is.
    
    PR-URL: #5756
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    780dd2d View commit details
    Browse the repository at this point in the history
  15. async_wrap: setupHooks now accepts object

    The number of callbacks accepted to setupHooks was getting unwieldy.
    Instead change the implementation to accept an object with all callbacks
    
    PR-URL: #5756
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    182b9de View commit details
    Browse the repository at this point in the history
  16. async_wrap: notify post if intercepted exception

    The second argument of the post callback is a boolean indicating whether
    the callback threw and was intercepted by uncaughtException or a domain.
    
    Currently node::MakeCallback has no way of retrieving a uid for the
    object. This is coming in a future patch.
    
    PR-URL: #5756
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    61dd47f View commit details
    Browse the repository at this point in the history
  17. async_wrap: don't abort on callback exception

    Rather than abort if the init/pre/post/final/destroy callbacks throw,
    force the exception to propagate and not be made catchable. This way
    the application is still not allowed to proceed but also allowed the
    location of the failure to print before exiting. Though the stack itself
    may not be of much use since all callbacks except init are called from
    the bottom of the call stack.
    
        /tmp/async-test.js:14
          throw new Error('pre');
          ^
        Error: pre
            at InternalFieldObject.pre (/tmp/async-test.js:14:9)
    
    PR-URL: #5756
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    8b1618a View commit details
    Browse the repository at this point in the history
  18. async_wrap: pass uid to JS as double

    Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
    will trim the value early. Instead use v8::Number::New() to convert the
    int64_t to a double so that JS can see the full 2^53 range of uid's.
    
    PR-URL: #7096
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
    trevnorris authored and AndreasMadsen committed Jun 7, 2016
    Configuration menu
    Copy the full SHA
    73c0b55 View commit details
    Browse the repository at this point in the history