{"action":"created","issue":{"url":"https://api.github.com/repos/Netflix/archaius/issues/468","repository_url":"https://api.github.com/repos/Netflix/archaius","labels_url":"https://api.github.com/repos/Netflix/archaius/issues/468/labels{/name}","comments_url":"https://api.github.com/repos/Netflix/archaius/issues/468/comments","events_url":"https://api.github.com/repos/Netflix/archaius/issues/468/events","html_url":"https://github.com/Netflix/archaius/issues/468","id":196172278,"number":468,"title":"Possible issues w/ PropertyWrapper?","user":{"login":"pkwarren","id":398111,"avatar_url":"https://avatars.githubusercontent.com/u/398111?v=3","gravatar_id":"","url":"https://api.github.com/users/pkwarren","html_url":"https://github.com/pkwarren","followers_url":"https://api.github.com/users/pkwarren/followers","following_url":"https://api.github.com/users/pkwarren/following{/other_user}","gists_url":"https://api.github.com/users/pkwarren/gists{/gist_id}","starred_url":"https://api.github.com/users/pkwarren/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pkwarren/subscriptions","organizations_url":"https://api.github.com/users/pkwarren/orgs","repos_url":"https://api.github.com/users/pkwarren/repos","events_url":"https://api.github.com/users/pkwarren/events{/privacy}","received_events_url":"https://api.github.com/users/pkwarren/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-12-16T22:55:15Z","updated_at":"2016-12-20T18:33:22Z","closed_at":null,"body":"When looking at the implementation of PropertyWrapper, I've seen two potential issues for consumers of this API:\r\n\r\n* `PropertyWrapper` wraps a `DynamicProperty` (which is thread safe), but appears to not be thread safe itself (specifically the `callbackList` field). It would be useful to document this (that `addCallback` and `removeAllCallbacks` should be synchronized) or perhaps switch the `callbackList` to a `CopyOnWriteArrayList` similar to `DynamicProperty`. Reference: https://github.com/Netflix/archaius/blob/e623c0131bdfa4a06a9cf62c2c2bc0a2e31f3ff7/archaius-core/src/main/java/com/netflix/config/PropertyWrapper.java#L41\r\n* In `removeAllCallbacks`, the callback is removed from `DynamicProperty` but the entry in `callbackList` is not removed. This will lead to a resource leak over time as there doesn't appear to be a way to remove callbacks from `callbackList`. Reference: https://github.com/Netflix/archaius/blob/e623c0131bdfa4a06a9cf62c2c2bc0a2e31f3ff7/archaius-core/src/main/java/com/netflix/config/PropertyWrapper.java#L156-L164\r\n\r\nI can raise a PR to resolve these issues."},"comment":{"url":"https://api.github.com/repos/Netflix/archaius/issues/comments/268320577","html_url":"https://github.com/Netflix/archaius/issues/468#issuecomment-268320577","issue_url":"https://api.github.com/repos/Netflix/archaius/issues/468","id":268320577,"user":{"login":"howardyuan","id":2171824,"avatar_url":"https://avatars.githubusercontent.com/u/2171824?v=3","gravatar_id":"","url":"https://api.github.com/users/howardyuan","html_url":"https://github.com/howardyuan","followers_url":"https://api.github.com/users/howardyuan/followers","following_url":"https://api.github.com/users/howardyuan/following{/other_user}","gists_url":"https://api.github.com/users/howardyuan/gists{/gist_id}","starred_url":"https://api.github.com/users/howardyuan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/howardyuan/subscriptions","organizations_url":"https://api.github.com/users/howardyuan/orgs","repos_url":"https://api.github.com/users/howardyuan/repos","events_url":"https://api.github.com/users/howardyuan/events{/privacy}","received_events_url":"https://api.github.com/users/howardyuan/received_events","type":"User","site_admin":false},"created_at":"2016-12-20T18:33:22Z","updated_at":"2016-12-20T18:33:22Z","body":"@pkwarren Sure, please raise a PR so we can look at it together. Thanks."}}
{"actor":{"display_login":"howardyuan"}}