Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • This project
    • Loading...
  • Sign in / Register
gjs
gjs
  • Overview
    • Overview
    • Details
    • Activity
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
  • Issues 20
    • Issues 20
    • List
    • Board
    • Labels
    • Milestones
  • Merge Requests 2
    • Merge Requests 2
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • GNOME
  • gjsgjs
  • Merge Requests
  • !27

Merged
Opened Dec 06, 2017 by Marco Trevisan@3v1n0 
  • Report abuse
Report abuse

Show errors when using proto functions on deleted object

In PR !22 (merged) I forgot to protect from accessing to invalid objects from proto functions.

Also mentioning if an object is finalized in the toString() function.

Edited Jan 05, 2018 by Marco Trevisan
×

Check out, review, and merge locally

Step 1. Fetch and check out the branch for this merge request

git fetch https://gitlab.gnome.org/3v1n0/gjs.git gobject-ward
git checkout -b 3v1n0/gjs-gobject-ward FETCH_HEAD

Step 2. Review the changes locally

Step 3. Merge the branch and fix any conflicts that come up

git checkout master
git merge --no-ff 3v1n0/gjs-gobject-ward

Step 4. Push the result of the merge to GitLab

git push origin master

Note that pushing to GitLab requires write access to this repository.

Tip: You can also checkout merge requests locally by following these guidelines.

  • Discussion 10
  • Commits 3
  • Pipelines 7
  • Changes 3
{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Marco Trevisan @3v1n0

    mentioned in merge request !28 (merged)

    Dec 06, 2017

    mentioned in merge request !28 (merged)

    mentioned in merge request !28
    Toggle commit list
  • Philip Chimento
    @ptomato started a discussion on an old version of the diff Dec 07, 2017
    Automatically resolved by Marco Trevisan with a push Dec 07, 2017
    installed-tests/js/testGObjectDestructionAccess.js
    21 }).toThrowError(/Object Gtk.Window \(0x[a-f0-9]+\), has been already finalized. Impossible to get any property from it./)
    21 }).toThrowError(/Object Gtk.Window \(0x[a-f0-9]+\), has been already finalized. Impossible to get any property from it\./)
    22 22 });
    23 23
    24 24 it('Set property', () => {
    25 25 expect(() => {
    26 26 destroyedWindow.title = 'I am dead';
    27 }).toThrowError(/Object Gtk.Window \(0x[a-f0-9]+\), has been already finalized. Impossible to set any property to it./)
    27 }).toThrowError(/Object Gtk.Window \(0x[a-f0-9]+\), has been already finalized. Impossible to set any property to it\./)
    28 28 });
    29 29
    30 30 it('Access to getter method', () => {
    31 31 expect(() => {
    32 32 let title = destroyedWindow.get_title();
    33 }).toThrowError(/Object Gtk.Window \(0x[a-f0-9]+\), has been already deallocated.*/)
    33 }).toThrowError(/Object Gtk.Window \(0x[a-f0-9]+\), has been already deallocated - impossible to access to it\..*/)
    • Philip Chimento @ptomato commented Dec 07, 2017
      Master

      Nitpick: .* isn't needed at the end of these regexes, since it will match anywhere in the string.

      Nitpick: `.*` isn't needed at the end of these regexes, since it will match anywhere in the string.
    • Marco Trevisan @3v1n0

      changed this line in version 2 of the diff

      Dec 07, 2017

      changed this line in version 2 of the diff

      changed this line in [version 2 of the diff](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=735&start_sha=45d85f1899dfb1e6ba80d572c653b6e2a9cc2b0a#10262a19c78add7cea0293ce960037cd650b46b9_33_33)
      Toggle commit list
    Please register or sign in to reply
  • Philip Chimento
    @ptomato started a discussion on the diff Dec 07, 2017
    Resolved by Marco Trevisan Dec 07, 2017
    gi/proxyutils.cpp
    63 63 g_string_append_printf(buf, " jsobj@%p", this_obj);
    64 64 if (native_address != NULL)
    65 65 g_string_append_printf(buf, " native@%p", native_address);
    66
    • Philip Chimento @ptomato commented Dec 07, 2017
      Master

      In theory this is a correct change, but if not changing anything else in this file, better to leave it out, or put it in a different commit.

      In theory this is a correct change, but if not changing anything else in this file, better to leave it out, or put it in a different commit.
    Please register or sign in to reply
  • Philip Chimento
    @ptomato started a discussion on an old version of the diff Dec 07, 2017
    Automatically resolved by Marco Trevisan with a push Dec 07, 2017
    gi/object.cpp
    1839 1860 return false; /* wrong class passed in */
    1840 1861 }
    1841 1862
    1842 return _gjs_proxy_to_string_func(context, obj, "object",
    1863 return _gjs_proxy_to_string_func(context, obj,
    1864 (priv->g_object_finalized) ?
    1865 "FINALIZED object" : "object",
    • Philip Chimento @ptomato commented Dec 07, 2017
      Master

      Not the most significant of code review comments, but [object (FINALIZED) ...] might be better in line with how other JS objects' toString() functions work. It also helps because I plan in the future to override Symbol.toStringTag rather than toString.

      Not the most significant of code review comments, but `[object (FINALIZED) ...]` might be better in line with how other JS objects' `toString()` functions work. It also helps because I plan in the future to override `Symbol.toStringTag` rather than `toString`.
    • Marco Trevisan @3v1n0

      changed this line in version 2 of the diff

      Dec 07, 2017

      changed this line in version 2 of the diff

      changed this line in [version 2 of the diff](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=735&start_sha=45d85f1899dfb1e6ba80d572c653b6e2a9cc2b0a#fd9d2d7a82fce609fc319a328f60bf4ee4bc26df_1865_1865)
      Toggle commit list
    Please register or sign in to reply
  • Philip Chimento @ptomato commented Dec 07, 2017
    Master

    Nice, thanks!

    Nice, thanks!
  • Marco Trevisan @3v1n0

    added 4 commits

    • c07ec39d - object: Throw error when using proto functions (connect*, emit) on destroyed object
    • c52469a2 - object: make clear in toString() proto method when the object is finalized
    • 4927c8d0 - testGObjectDestructionAccess: use better regex checks
    • 9236f56e - proxyutils: remove trailing spaces

    Compare with previous version

    Dec 07, 2017

    added 4 commits

    • c07ec39d - object: Throw error when using proto functions (connect*, emit) on destroyed object
    • c52469a2 - object: make clear in toString() proto method when the object is finalized
    • 4927c8d0 - testGObjectDestructionAccess: use better regex checks
    • 9236f56e - proxyutils: remove trailing spaces

    Compare with previous version

    added 4 commits * c07ec39d - object: Throw error when using proto functions (connect*, emit) on destroyed object * c52469a2 - object: make clear in toString() proto method when the object is finalized * 4927c8d0 - testGObjectDestructionAccess: use better regex checks * 9236f56e - proxyutils: remove trailing spaces [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=735&start_sha=45d85f1899dfb1e6ba80d572c653b6e2a9cc2b0a)
    Toggle commit list
  • Marco Trevisan @3v1n0

    resolved all discussions

    Dec 07, 2017

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Philip Chimento @ptomato commented Dec 09, 2017
    Master

    👍 Would you mind squashing the trailing spaces commit into the commit that originally introduced the whitespace change?

    Other than that I agree with it, but it would be best to get #21 (closed) fixed before merging this.

    :+1: Would you mind squashing the trailing spaces commit into the commit that originally introduced the whitespace change? Other than that I agree with it, but it would be best to get #21 fixed before merging this.
  • Marco Trevisan @3v1n0 commented Dec 11, 2017
    Developer

    Would you mind squashing the trailing spaces commit into the commit that originally introduced the whitespace change?

    Sorry, i'm not sure I got it... The whitespace change was introduced in commit 58cb9cd2, with what commit should I squash it?

    Other than that I agree with it, but it would be best to get #21 (closed) fixed before merging this.

    Ack, let me prepare a PR.

    > Would you mind squashing the trailing spaces commit into the commit that originally introduced the whitespace change? Sorry, i'm not sure I got it... The whitespace change was introduced in commit 58cb9cd2, with what commit should I squash it? > Other than that I agree with it, but it would be best to get #21 fixed before merging this. Ack, let me prepare a PR.
  • Philip Chimento @ptomato commented Dec 24, 2017
    Master

    Would you mind squashing the trailing spaces commit into the commit that originally introduced the whitespace change?

    Sorry, i'm not sure I got it... The whitespace change was introduced in commit 58cb9cd2, with what commit should I squash it?

    Looks like it's been resolved in a rebase anyway.

    > > Would you mind squashing the trailing spaces commit into the commit that originally introduced the whitespace change? > > Sorry, i'm not sure I got it... The whitespace change was introduced in commit 58cb9cd2, with what commit should I squash it? Looks like it's been resolved in a rebase anyway.
  • Marco Trevisan @3v1n0

    resolved all discussions

    Dec 28, 2017

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Marco Trevisan @3v1n0

    added 18 commits

    • 9236f56e...617257d3 - 4 commits from branch GNOME:master
    • 718ca0bc - CI: add code coverage test
    • 5b340f2f - CI: add code coverage test
    • 5cdc826a - One more test
    • 9914af79 - Amend gtk-application.js
    • 40e023ca - mv gtk-window.js to amend gtk.js, including suggestions
    • 7efb9ff8 - arg: String as GdkAtom, and GdkAtom as string
    • 19505e5d - CI: do not exit abruptly if files are different
    • 8c8f8a4a - CI: add the coverage badge
    • b7817e54 - CI: tweak the docker image
    • 0768e54f - object: Only show critical errors on access to finalized objects
    • faefa0b9 - Test case improved in installed-tests/js/testCairo.js [fixes #27 (closed)]
    • e537eb06 - object: Throw error when using proto functions (connect*, emit) on destroyed object
    • e3db3a1b - object: make clear in toString() proto method when the object is finalized
    • 3d6084a4 - proxyutils: remove trailing spaces

    Compare with previous version

    Dec 28, 2017

    added 18 commits

    • 9236f56e...617257d3 - 4 commits from branch GNOME:master
    • 718ca0bc - CI: add code coverage test
    • 5b340f2f - CI: add code coverage test
    • 5cdc826a - One more test
    • 9914af79 - Amend gtk-application.js
    • 40e023ca - mv gtk-window.js to amend gtk.js, including suggestions
    • 7efb9ff8 - arg: String as GdkAtom, and GdkAtom as string
    • 19505e5d - CI: do not exit abruptly if files are different
    • 8c8f8a4a - CI: add the coverage badge
    • b7817e54 - CI: tweak the docker image
    • 0768e54f - object: Only show critical errors on access to finalized objects
    • faefa0b9 - Test case improved in installed-tests/js/testCairo.js [fixes #27 (closed)]
    • e537eb06 - object: Throw error when using proto functions (connect*, emit) on destroyed object
    • e3db3a1b - object: make clear in toString() proto method when the object is finalized
    • 3d6084a4 - proxyutils: remove trailing spaces

    Compare with previous version

    added 18 commits * 9236f56e...617257d3 - 4 commits from branch `GNOME:master` * 718ca0bc - CI: add code coverage test * 5b340f2f - CI: add code coverage test * 5cdc826a - One more test * 9914af79 - Amend gtk-application.js * 40e023ca - mv gtk-window.js to amend gtk.js, including suggestions * 7efb9ff8 - arg: String as GdkAtom, and GdkAtom as string * 19505e5d - CI: do not exit abruptly if files are different * 8c8f8a4a - CI: add the coverage badge * b7817e54 - CI: tweak the docker image * 0768e54f - object: Only show critical errors on access to finalized objects * faefa0b9 - Test case improved in `installed-tests/js/testCairo.js` [fixes #27] * e537eb06 - object: Throw error when using proto functions (connect*, emit) on destroyed object * e3db3a1b - object: make clear in toString() proto method when the object is finalized * 3d6084a4 - proxyutils: remove trailing spaces [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=1035&start_sha=9236f56ef7a6c648b08acd539890f5a02a8a81df)
    Toggle commit list
  • Marco Trevisan @3v1n0

    resolved all discussions

    Dec 28, 2017

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Marco Trevisan @3v1n0

    added 19 commits

    • 3d6084a4...e2986711 - 16 commits from branch GNOME:master
    • 3b95ff5a - object: Throw error when using proto functions (connect*, emit) on destroyed object
    • 984fcae4 - object: make clear in toString() proto method when the object is finalized
    • f85278bb - proxyutils: remove trailing spaces

    Compare with previous version

    Dec 28, 2017

    added 19 commits

    • 3d6084a4...e2986711 - 16 commits from branch GNOME:master
    • 3b95ff5a - object: Throw error when using proto functions (connect*, emit) on destroyed object
    • 984fcae4 - object: make clear in toString() proto method when the object is finalized
    • f85278bb - proxyutils: remove trailing spaces

    Compare with previous version

    added 19 commits * 3d6084a4...e2986711 - 16 commits from branch `GNOME:master` * 3b95ff5a - object: Throw error when using proto functions (connect*, emit) on destroyed object * 984fcae4 - object: make clear in toString() proto method when the object is finalized * f85278bb - proxyutils: remove trailing spaces [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=1037&start_sha=3d6084a448da9b1542147c73d7d873fbfa3ce5ae)
    Toggle commit list
  • Marco Trevisan @3v1n0 commented Dec 28, 2017
    Developer

    Ok, rebased... I'm confident that in this case keeping the throw is fine, as such operations are easier to track, don't you agree?

    Ok, rebased... I'm confident that in this case keeping the throw is fine, as such operations are easier to track, don't you agree?
  • Philip Chimento @ptomato commented Dec 29, 2017
    Master

    Looks good, but I'd prefer to keep it consistently to a log message now that we've switched to a log message on the other operations. At least until we're sure that #24 is not a problem anymore.

    Looks good, but I'd prefer to keep it consistently to a log message now that we've switched to a log message on the other operations. At least until we're sure that #24 is not a problem anymore.
  • Marco Trevisan @3v1n0

    resolved all discussions

    Jan 05, 2018

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Marco Trevisan @3v1n0

    added 6 commits

    • f85278bb...9339f85d - 3 commits from branch GNOME:master
    • 8a7566a8 - object: Show error when using proto functions (connect*, emit) on destroyed object
    • 311f03ac - object: make clear in toString() proto method when the object is finalized
    • ac08b9f9 - proxyutils: remove trailing spaces

    Compare with previous version

    Jan 05, 2018

    added 6 commits

    • f85278bb...9339f85d - 3 commits from branch GNOME:master
    • 8a7566a8 - object: Show error when using proto functions (connect*, emit) on destroyed object
    • 311f03ac - object: make clear in toString() proto method when the object is finalized
    • ac08b9f9 - proxyutils: remove trailing spaces

    Compare with previous version

    added 6 commits * f85278bb...9339f85d - 3 commits from branch `GNOME:master` * 8a7566a8 - object: Show error when using proto functions (connect*, emit) on destroyed object * 311f03ac - object: make clear in toString() proto method when the object is finalized * ac08b9f9 - proxyutils: remove trailing spaces [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=1101&start_sha=f85278bb4213f55273aab9a93eaa24e4ed64ecdf)
    Toggle commit list
  • Marco Trevisan @3v1n0

    changed title from Throw errors when using proto functions on deleted object to Show errors when using proto functions on deleted object

    Jan 05, 2018

    changed title from Throw errors when using proto functions on deleted object to Show errors when using proto functions on deleted object

    changed title from **{-Thr-}ow errors when using proto functions on deleted object** to **{+Sh+}ow errors when using proto functions on deleted object**
    Toggle commit list
  • Marco Trevisan @3v1n0

    changed the description

    Jan 05, 2018

    changed the description

    changed the description
    Toggle commit list
  • Marco Trevisan @3v1n0

    resolved all discussions

    Jan 05, 2018

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Marco Trevisan @3v1n0

    added 16 commits

    • 1555e2fa - CI: add code coverage test
    • 3a716844 - CI: add code coverage test
    • 99d49ce3 - One more test
    • 25a1b215 - Amend gtk-application.js
    • 1fc1e41d - mv gtk-window.js to amend gtk.js, including suggestions
    • b60bb79c - arg: String as GdkAtom, and GdkAtom as string
    • 7bf1dd7d - CI: do not exit abruptly if files are different
    • 592940cb - CI: add the coverage badge
    • 7dbe567e - CI: tweak the docker image
    • 0eeb15a3 - object: Only show critical errors on access to finalized objects
    • f42b16ad - Test case improved in installed-tests/js/testCairo.js [fixes #27 (closed)]
    • 7c5c7768 - coverage: Remove unused JSCompartmentOptions
    • d5cc88d4 - js: Remove unnecessary property getters and setters
    • 40e38302 - object: Show error when using proto functions (connect*, emit) on destroyed object
    • 00df5b74 - object: make clear in toString() proto method when the object is finalized
    • 0d7816a2 - proxyutils: remove trailing spaces

    Compare with previous version

    Jan 05, 2018

    added 16 commits

    • 1555e2fa - CI: add code coverage test
    • 3a716844 - CI: add code coverage test
    • 99d49ce3 - One more test
    • 25a1b215 - Amend gtk-application.js
    • 1fc1e41d - mv gtk-window.js to amend gtk.js, including suggestions
    • b60bb79c - arg: String as GdkAtom, and GdkAtom as string
    • 7bf1dd7d - CI: do not exit abruptly if files are different
    • 592940cb - CI: add the coverage badge
    • 7dbe567e - CI: tweak the docker image
    • 0eeb15a3 - object: Only show critical errors on access to finalized objects
    • f42b16ad - Test case improved in installed-tests/js/testCairo.js [fixes #27 (closed)]
    • 7c5c7768 - coverage: Remove unused JSCompartmentOptions
    • d5cc88d4 - js: Remove unnecessary property getters and setters
    • 40e38302 - object: Show error when using proto functions (connect*, emit) on destroyed object
    • 00df5b74 - object: make clear in toString() proto method when the object is finalized
    • 0d7816a2 - proxyutils: remove trailing spaces

    Compare with previous version

    added 16 commits * 1555e2fa - CI: add code coverage test * 3a716844 - CI: add code coverage test * 99d49ce3 - One more test * 25a1b215 - Amend gtk-application.js * 1fc1e41d - mv gtk-window.js to amend gtk.js, including suggestions * b60bb79c - arg: String as GdkAtom, and GdkAtom as string * 7bf1dd7d - CI: do not exit abruptly if files are different * 592940cb - CI: add the coverage badge * 7dbe567e - CI: tweak the docker image * 0eeb15a3 - object: Only show critical errors on access to finalized objects * f42b16ad - Test case improved in `installed-tests/js/testCairo.js` [fixes #27] * 7c5c7768 - coverage: Remove unused JSCompartmentOptions * d5cc88d4 - js: Remove unnecessary property getters and setters * 40e38302 - object: Show error when using proto functions (connect*, emit) on destroyed object * 00df5b74 - object: make clear in toString() proto method when the object is finalized * 0d7816a2 - proxyutils: remove trailing spaces [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=1102&start_sha=ac08b9f96f81e3b4ff4cbea03a2b2414794315f0)
    Toggle commit list
  • Marco Trevisan @3v1n0

    resolved all discussions

    Jan 05, 2018

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Marco Trevisan @3v1n0

    added 22 commits

    • 0d7816a2...9339f85d - 19 commits from branch GNOME:master
    • 8a7566a8 - object: Show error when using proto functions (connect*, emit) on destroyed object
    • 29cd1f2e - object: make clear in toString() proto method when the object is finalized
    • e49dbaba - proxyutils: remove trailing spaces

    Compare with previous version

    Jan 05, 2018

    added 22 commits

    • 0d7816a2...9339f85d - 19 commits from branch GNOME:master
    • 8a7566a8 - object: Show error when using proto functions (connect*, emit) on destroyed object
    • 29cd1f2e - object: make clear in toString() proto method when the object is finalized
    • e49dbaba - proxyutils: remove trailing spaces

    Compare with previous version

    added 22 commits * 0d7816a2...9339f85d - 19 commits from branch `GNOME:master` * 8a7566a8 - object: Show error when using proto functions (connect*, emit) on destroyed object * 29cd1f2e - object: make clear in toString() proto method when the object is finalized * e49dbaba - proxyutils: remove trailing spaces [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/27/diffs?diff_id=1103&start_sha=0d7816a2435aecf79217f0ddb187145fc48ce0e1)
    Toggle commit list
  • Philip Chimento @ptomato

    mentioned in commit c944334f

    Jan 06, 2018

    mentioned in commit c944334f

    mentioned in commit c944334f362243dfa874b27a67ef8de3db0c17ae
    Toggle commit list
  • Philip Chimento @ptomato

    merged

    Jan 06, 2018

    merged

    merged
    Toggle commit list
  • Philip Chimento @ptomato commented Jan 06, 2018
    Master

    Thanks!

    Thanks!
  • Write
  • Preview
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
Philip Chimento
Assignee
Philip Chimento @ptomato
Assign to
None
Milestone
None
Assign milestone
Time tracking
0
Labels
None
Assign labels
  • View labels
Reference: GNOME/gjs!27
×

Revert this merge request

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.
×

Cherry-pick this merge request

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.