You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Cliff Jansen <cl...@gmail.com> on 2014/07/02 18:18:16 UTC
Re: Review Request 23122: Proton map/hash entries can disappear
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23122/
-----------------------------------------------------------
(Updated July 2, 2014, 4:18 p.m.)
Review request for qpid and Rafael Schloming.
Changes
-------
The test case I had wasn't generic (it was based on actual data that would unreliably fail, depending on heisenburg's mood), so I did not include it in the regression testing.
The first comment gave a clue how to make a generic test in the first place, so that's mostly what this refresh is about.
The original code is unchanged except fixing the bogus assert (ick).
Bugs: PROTON-617
https://issues.apache.org/jira/browse/PROTON-617
Repository: qpid
Description
-------
The proposed fix tests for the case of being the first link in a multipart chain and copies the second entry over top and makes the old location of the second entry available for reuse.
Diffs (updated)
-----
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/object/object.c 1607407
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/tests/object.c 1607407
Diff: https://reviews.apache.org/r/23122/diff/
Testing
-------
Thanks,
Cliff Jansen
Re: Review Request 23122: Proton map/hash entries can disappear
Posted by Rafael Schloming <rh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23122/#review47208
-----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/object/object.c
<https://reviews.apache.org/r/23122/#comment82848>
I think strictly speaking we should copy these pointers out and move the decrefs to the end of this function. It is hypothetically possible that one or both of the decrefs could trigger a finalize which in turn re-enters this code by deleting from the map. To deal with that case we probably want the hashtable data structures to be in a fully modified and in a consistent state before calling decref.
- Rafael Schloming
On July 2, 2014, 4:18 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23122/
> -----------------------------------------------------------
>
> (Updated July 2, 2014, 4:18 p.m.)
>
>
> Review request for qpid and Rafael Schloming.
>
>
> Bugs: PROTON-617
> https://issues.apache.org/jira/browse/PROTON-617
>
>
> Repository: qpid
>
>
> Description
> -------
>
> The proposed fix tests for the case of being the first link in a multipart chain and copies the second entry over top and makes the old location of the second entry available for reuse.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/object/object.c 1607407
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/tests/object.c 1607407
>
> Diff: https://reviews.apache.org/r/23122/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request 23122: Proton map/hash entries can disappear
Posted by Rafael Schloming <rh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23122/#review47224
-----------------------------------------------------------
Ship it!
Ship It!
- Rafael Schloming
On July 2, 2014, 7:06 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23122/
> -----------------------------------------------------------
>
> (Updated July 2, 2014, 7:06 p.m.)
>
>
> Review request for qpid and Rafael Schloming.
>
>
> Bugs: PROTON-617
> https://issues.apache.org/jira/browse/PROTON-617
>
>
> Repository: qpid
>
>
> Description
> -------
>
> The proposed fix tests for the case of being the first link in a multipart chain and copies the second entry over top and makes the old location of the second entry available for reuse.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/object/object.c 1607407
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/tests/object.c 1607407
>
> Diff: https://reviews.apache.org/r/23122/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request 23122: Proton map/hash entries can disappear
Posted by Cliff Jansen <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23122/
-----------------------------------------------------------
(Updated July 2, 2014, 7:06 p.m.)
Review request for qpid and Rafael Schloming.
Changes
-------
revised to decref last, without reference to the map itself, as suggested
Bugs: PROTON-617
https://issues.apache.org/jira/browse/PROTON-617
Repository: qpid
Description
-------
The proposed fix tests for the case of being the first link in a multipart chain and copies the second entry over top and makes the old location of the second entry available for reuse.
Diffs (updated)
-----
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/object/object.c 1607407
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/tests/object.c 1607407
Diff: https://reviews.apache.org/r/23122/diff/
Testing
-------
Thanks,
Cliff Jansen