{"action":"created","comment":{"url":"https://api.github.com/repos/pharo-project/pharo/pulls/comments/294070595","pull_request_review_id":250218457,"id":294070595,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDI5NDA3MDU5NQ==","diff_hunk":"@@ -0,0 +1,216 @@\n+Class {\n+\t#name : #WaitfreeQueueTest,\n+\t#superclass : #TestCase,\n+\t#category : #'Collections-Tests-Atomic'\n+}\n+\n+{ #category : #'helper methods' }\n+WaitfreeQueueTest >> newItems: count [\n+\t\"Answer a collection with the requested number of items\"\n+\n+\t| collection |\n+\n+\tcollection := OrderedCollection new.\n+\tcount timesRepeat: [ collection add: AtomicQueueItem new ].\n+\t^collection\n+]\n+\n+{ #category : #'helper methods' }\n+WaitfreeQueueTest >> queueFromCollection: aCollection [\n+\t\"Answer a WaitfreeQueue initialised from the supplied collection of items\"\n+\n+\t| queue |\n+\n+\tqueue := WaitfreeQueue new.\n+\t(queue instVarNamed: #dummy) next: aCollection first.","path":"src/Collections-Tests/WaitfreeQueueTest.class.st","position":25,"original_position":25,"commit_id":"9608f71108926ae83cbe3da3d948d39f2e9f9b82","original_commit_id":"9608f71108926ae83cbe3da3d948d39f2e9f9b82","user":{"login":"akgrant43","id":2062166,"node_id":"MDQ6VXNlcjIwNjIxNjY=","avatar_url":"https://avatars0.githubusercontent.com/u/2062166?v=4","gravatar_id":"","url":"https://api.github.com/users/akgrant43","html_url":"https://github.com/akgrant43","followers_url":"https://api.github.com/users/akgrant43/followers","following_url":"https://api.github.com/users/akgrant43/following{/other_user}","gists_url":"https://api.github.com/users/akgrant43/gists{/gist_id}","starred_url":"https://api.github.com/users/akgrant43/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akgrant43/subscriptions","organizations_url":"https://api.github.com/users/akgrant43/orgs","repos_url":"https://api.github.com/users/akgrant43/repos","events_url":"https://api.github.com/users/akgrant43/events{/privacy}","received_events_url":"https://api.github.com/users/akgrant43/received_events","type":"User","site_admin":false},"body":"Hi Denis,\r\n\r\n> Can we avoid instVarNamed?\r\n> It breaks encapsulation and code navigation. No way to find those methods using var references\r\n\r\nI'm interested in your thoughts here.  There's two rules that both make sense:\r\n\r\n1. Don't access internal state using `#instVarNamed:` for the reasons you outline above, as well as being slow.\r\n2. Don't expose internal state that other objects shouldn't have access to (the dummy variable in this case).\r\n\r\nI try to balance this by not using `#instVarNamed:` in production code, but using it for setting up internal state in associated tests (which are intimately tied to the tested class).\r\n\r\nI can add a #dummy access method, but it will the be breaking the second rule.","created_at":"2019-06-16T07:02:02Z","updated_at":"2019-06-16T07:02:02Z","html_url":"https://github.com/pharo-project/pharo/pull/3205#discussion_r294070595","pull_request_url":"https://api.github.com/repos/pharo-project/pharo/pulls/3205","author_association":"COLLABORATOR","_links":{"self":{"href":"https://api.github.com/repos/pharo-project/pharo/pulls/comments/294070595"},"html":{"href":"https://github.com/pharo-project/pharo/pull/3205#discussion_r294070595"},"pull_request":{"href":"https://api.github.com/repos/pharo-project/pharo/pulls/3205"}},"in_reply_to_id":292789392},"pull_request":{"url":"https://api.github.com/repos/pharo-project/pharo/pulls/3205","id":271727799,"node_id":"MDExOlB1bGxSZXF1ZXN0MjcxNzI3Nzk5","html_url":"https://github.com/pharo-project/pharo/pull/3205","diff_url":"https://github.com/pharo-project/pharo/pull/3205.diff","patch_url":"https://github.com/pharo-project/pharo/pull/3205.patch","issue_url":"https://api.github.com/repos/pharo-project/pharo/issues/3205","number":3205,"state":"open","locked":false,"title":"3198-WaitfreeQeuenextOrNilSuchThat-memory-leak","user":{"login":"akgrant43","id":2062166,"node_id":"MDQ6VXNlcjIwNjIxNjY=","avatar_url":"https://avatars0.githubusercontent.com/u/2062166?v=4","gravatar_id":"","url":"https://api.github.com/users/akgrant43","html_url":"https://github.com/akgrant43","followers_url":"https://api.github.com/users/akgrant43/followers","following_url":"https://api.github.com/users/akgrant43/following{/other_user}","gists_url":"https://api.github.com/users/akgrant43/gists{/gist_id}","starred_url":"https://api.github.com/users/akgrant43/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akgrant43/subscriptions","organizations_url":"https://api.github.com/users/akgrant43/orgs","repos_url":"https://api.github.com/users/akgrant43/repos","events_url":"https://api.github.com/users/akgrant43/events{/privacy}","received_events_url":"https://api.github.com/users/akgrant43/received_events","type":"User","site_admin":false},"body":"WaitfreeQueue>>nextOrNilSuchThat: currently never removes zombie queue items, i.e. an AtomicQueueItem where the object points back to the dummy entry.  This means the list of items potentially grows indefinitely unless #nextOrNil is called enough times to empty the queue, wasting memory and CPU cycles.\n\n- Modify #nextOrNilSuchThat: to remove zombie list items (unless it is the last item in the list).\n- Update the class comments to describe how thread-safety is achieved and the constraints imposed as part of thread-safety.\n- Add automated tests.\n\nAdditional details on tracking the issue down and figuring out the appropriate behaviour at: https://github.com/feenkcom/gtoolkit/issues/244\n\nFixes: #3198","created_at":"2019-04-18T15:09:15Z","updated_at":"2019-06-16T07:02:02Z","closed_at":null,"merged_at":null,"merge_commit_sha":"b38c3dcaa5ed15dd2635e4891226af863a53a3e4","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":1183082009,"node_id":"MDU6TGFiZWwxMTgzMDgyMDA5","url":"https://api.github.com/repos/pharo-project/pharo/labels/review","name":"review","color":"ededed","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/pharo-project/pharo/pulls/3205/commits","review_comments_url":"https://api.github.com/repos/pharo-project/pharo/pulls/3205/comments","review_comment_url":"https://api.github.com/repos/pharo-project/pharo/pulls/comments{/number}","comments_url":"https://api.github.com/repos/pharo-project/pharo/issues/3205/comments","statuses_url":"https://api.github.com/repos/pharo-project/pharo/statuses/9608f71108926ae83cbe3da3d948d39f2e9f9b82","head":{"label":"akgrant43:3198-WaitfreeQeuenextOrNilSuchThat-memory-leak","ref":"3198-WaitfreeQeuenextOrNilSuchThat-memory-leak","sha":"9608f71108926ae83cbe3da3d948d39f2e9f9b82","user":{"login":"akgrant43","id":2062166,"node_id":"MDQ6VXNlcjIwNjIxNjY=","avatar_url":"https://avatars0.githubusercontent.com/u/2062166?v=4","gravatar_id":"","url":"https://api.github.com/users/akgrant43","html_url":"https://github.com/akgrant43","followers_url":"https://api.github.com/users/akgrant43/followers","following_url":"https://api.github.com/users/akgrant43/following{/other_user}","gists_url":"https://api.github.com/users/akgrant43/gists{/gist_id}","starred_url":"https://api.github.com/users/akgrant43/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akgrant43/subscriptions","organizations_url":"https://api.github.com/users/akgrant43/orgs","repos_url":"https://api.github.com/users/akgrant43/repos","events_url":"https://api.github.com/users/akgrant43/events{/privacy}","received_events_url":"https://api.github.com/users/akgrant43/received_events","type":"User","site_admin":false},"repo":{"id":114294968,"node_id":"MDEwOlJlcG9zaXRvcnkxMTQyOTQ5Njg=","name":"pharo","full_name":"akgrant43/pharo","private":false,"owner":{"login":"akgrant43","id":2062166,"node_id":"MDQ6VXNlcjIwNjIxNjY=","avatar_url":"https://avatars0.githubusercontent.com/u/2062166?v=4","gravatar_id":"","url":"https://api.github.com/users/akgrant43","html_url":"https://github.com/akgrant43","followers_url":"https://api.github.com/users/akgrant43/followers","following_url":"https://api.github.com/users/akgrant43/following{/other_user}","gists_url":"https://api.github.com/users/akgrant43/gists{/gist_id}","starred_url":"https://api.github.com/users/akgrant43/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akgrant43/subscriptions","organizations_url":"https://api.github.com/users/akgrant43/orgs","repos_url":"https://api.github.com/users/akgrant43/repos","events_url":"https://api.github.com/users/akgrant43/events{/privacy}","received_events_url":"https://api.github.com/users/akgrant43/received_events","type":"User","site_admin":false},"html_url":"https://github.com/akgrant43/pharo","description":"The sources for Pharo","fork":true,"url":"https://api.github.com/repos/akgrant43/pharo","forks_url":"https://api.github.com/repos/akgrant43/pharo/forks","keys_url":"https://api.github.com/repos/akgrant43/pharo/keys{/key_id}","collaborators_url":"https://api.github.com/repos/akgrant43/pharo/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/akgrant43/pharo/teams","hooks_url":"https://api.github.com/repos/akgrant43/pharo/hooks","issue_events_url":"https://api.github.com/repos/akgrant43/pharo/issues/events{/number}","events_url":"https://api.github.com/repos/akgrant43/pharo/events","assignees_url":"https://api.github.com/repos/akgrant43/pharo/assignees{/user}","branches_url":"https://api.github.com/repos/akgrant43/pharo/branches{/branch}","tags_url":"https://api.github.com/repos/akgrant43/pharo/tags","blobs_url":"https://api.github.com/repos/akgrant43/pharo/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/akgrant43/pharo/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/akgrant43/pharo/git/refs{/sha}","trees_url":"https://api.github.com/repos/akgrant43/pharo/git/trees{/sha}","statuses_url":"https://api.github.com/repos/akgrant43/pharo/statuses/{sha}","languages_url":"https://api.github.com/repos/akgrant43/pharo/languages","stargazers_url":"https://api.github.com/repos/akgrant43/pharo/stargazers","contributors_url":"https://api.github.com/repos/akgrant43/pharo/contributors","subscribers_url":"https://api.github.com/repos/akgrant43/pharo/subscribers","subscription_url":"https://api.github.com/repos/akgrant43/pharo/subscription","commits_url":"https://api.github.com/repos/akgrant43/pharo/commits{/sha}","git_commits_url":"https://api.github.com/repos/akgrant43/pharo/git/commits{/sha}","comments_url":"https://api.github.com/repos/akgrant43/pharo/comments{/number}","issue_comment_url":"https://api.github.com/repos/akgrant43/pharo/issues/comments{/number}","contents_url":"https://api.github.com/repos/akgrant43/pharo/contents/{+path}","compare_url":"https://api.github.com/repos/akgrant43/pharo/compare/{base}...{head}","merges_url":"https://api.github.com/repos/akgrant43/pharo/merges","archive_url":"https://api.github.com/repos/akgrant43/pharo/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/akgrant43/pharo/downloads","issues_url":"https://api.github.com/repos/akgrant43/pharo/issues{/number}","pulls_url":"https://api.github.com/repos/akgrant43/pharo/pulls{/number}","milestones_url":"https://api.github.com/repos/akgrant43/pharo/milestones{/number}","notifications_url":"https://api.github.com/repos/akgrant43/pharo/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/akgrant43/pharo/labels{/name}","releases_url":"https://api.github.com/repos/akgrant43/pharo/releases{/id}","deployments_url":"https://api.github.com/repos/akgrant43/pharo/deployments","created_at":"2017-12-14T20:49:50Z","updated_at":"2019-03-01T15:01:53Z","pushed_at":"2019-05-23T06:30:14Z","git_url":"git://github.com/akgrant43/pharo.git","ssh_url":"git@github.com:akgrant43/pharo.git","clone_url":"https://github.com/akgrant43/pharo.git","svn_url":"https://github.com/akgrant43/pharo","homepage":"http://pharo.org","size":51313,"stargazers_count":0,"watchers_count":0,"language":"Smalltalk","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":0,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit","node_id":"MDc6TGljZW5zZTEz"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"Pharo8.0"}},"base":{"label":"pharo-project:Pharo8.0","ref":"Pharo8.0","sha":"92cbc0f95eb42df09b2398d74166f53edbd94a72","user":{"login":"pharo-project","id":1838382,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE4MzgzODI=","avatar_url":"https://avatars3.githubusercontent.com/u/1838382?v=4","gravatar_id":"","url":"https://api.github.com/users/pharo-project","html_url":"https://github.com/pharo-project","followers_url":"https://api.github.com/users/pharo-project/followers","following_url":"https://api.github.com/users/pharo-project/following{/other_user}","gists_url":"https://api.github.com/users/pharo-project/gists{/gist_id}","starred_url":"https://api.github.com/users/pharo-project/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pharo-project/subscriptions","organizations_url":"https://api.github.com/users/pharo-project/orgs","repos_url":"https://api.github.com/users/pharo-project/repos","events_url":"https://api.github.com/users/pharo-project/events{/privacy}","received_events_url":"https://api.github.com/users/pharo-project/received_events","type":"Organization","site_admin":false},"repo":{"id":64123480,"node_id":"MDEwOlJlcG9zaXRvcnk2NDEyMzQ4MA==","name":"pharo","full_name":"pharo-project/pharo","private":false,"owner":{"login":"pharo-project","id":1838382,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE4MzgzODI=","avatar_url":"https://avatars3.githubusercontent.com/u/1838382?v=4","gravatar_id":"","url":"https://api.github.com/users/pharo-project","html_url":"https://github.com/pharo-project","followers_url":"https://api.github.com/users/pharo-project/followers","following_url":"https://api.github.com/users/pharo-project/following{/other_user}","gists_url":"https://api.github.com/users/pharo-project/gists{/gist_id}","starred_url":"https://api.github.com/users/pharo-project/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pharo-project/subscriptions","organizations_url":"https://api.github.com/users/pharo-project/orgs","repos_url":"https://api.github.com/users/pharo-project/repos","events_url":"https://api.github.com/users/pharo-project/events{/privacy}","received_events_url":"https://api.github.com/users/pharo-project/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/pharo-project/pharo","description":"The Sources for Pharo","fork":false,"url":"https://api.github.com/repos/pharo-project/pharo","forks_url":"https://api.github.com/repos/pharo-project/pharo/forks","keys_url":"https://api.github.com/repos/pharo-project/pharo/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pharo-project/pharo/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pharo-project/pharo/teams","hooks_url":"https://api.github.com/repos/pharo-project/pharo/hooks","issue_events_url":"https://api.github.com/repos/pharo-project/pharo/issues/events{/number}","events_url":"https://api.github.com/repos/pharo-project/pharo/events","assignees_url":"https://api.github.com/repos/pharo-project/pharo/assignees{/user}","branches_url":"https://api.github.com/repos/pharo-project/pharo/branches{/branch}","tags_url":"https://api.github.com/repos/pharo-project/pharo/tags","blobs_url":"https://api.github.com/repos/pharo-project/pharo/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pharo-project/pharo/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pharo-project/pharo/git/refs{/sha}","trees_url":"https://api.github.com/repos/pharo-project/pharo/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pharo-project/pharo/statuses/{sha}","languages_url":"https://api.github.com/repos/pharo-project/pharo/languages","stargazers_url":"https://api.github.com/repos/pharo-project/pharo/stargazers","contributors_url":"https://api.github.com/repos/pharo-project/pharo/contributors","subscribers_url":"https://api.github.com/repos/pharo-project/pharo/subscribers","subscription_url":"https://api.github.com/repos/pharo-project/pharo/subscription","commits_url":"https://api.github.com/repos/pharo-project/pharo/commits{/sha}","git_commits_url":"https://api.github.com/repos/pharo-project/pharo/git/commits{/sha}","comments_url":"https://api.github.com/repos/pharo-project/pharo/comments{/number}","issue_comment_url":"https://api.github.com/repos/pharo-project/pharo/issues/comments{/number}","contents_url":"https://api.github.com/repos/pharo-project/pharo/contents/{+path}","compare_url":"https://api.github.com/repos/pharo-project/pharo/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pharo-project/pharo/merges","archive_url":"https://api.github.com/repos/pharo-project/pharo/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pharo-project/pharo/downloads","issues_url":"https://api.github.com/repos/pharo-project/pharo/issues{/number}","pulls_url":"https://api.github.com/repos/pharo-project/pharo/pulls{/number}","milestones_url":"https://api.github.com/repos/pharo-project/pharo/milestones{/number}","notifications_url":"https://api.github.com/repos/pharo-project/pharo/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pharo-project/pharo/labels{/name}","releases_url":"https://api.github.com/repos/pharo-project/pharo/releases{/id}","deployments_url":"https://api.github.com/repos/pharo-project/pharo/deployments","created_at":"2016-07-25T09:55:28Z","updated_at":"2019-06-15T11:52:00Z","pushed_at":"2019-06-15T05:02:38Z","git_url":"git://github.com/pharo-project/pharo.git","ssh_url":"git@github.com:pharo-project/pharo.git","clone_url":"https://github.com/pharo-project/pharo.git","svn_url":"https://github.com/pharo-project/pharo","homepage":"http://pharo.org","size":51987,"stargazers_count":319,"watchers_count":319,"language":"Smalltalk","has_issues":true,"has_projects":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":158,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":437,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit","node_id":"MDc6TGljZW5zZTEz"},"forks":158,"open_issues":437,"watchers":319,"default_branch":"Pharo8.0"}},"_links":{"self":{"href":"https://api.github.com/repos/pharo-project/pharo/pulls/3205"},"html":{"href":"https://github.com/pharo-project/pharo/pull/3205"},"issue":{"href":"https://api.github.com/repos/pharo-project/pharo/issues/3205"},"comments":{"href":"https://api.github.com/repos/pharo-project/pharo/issues/3205/comments"},"review_comments":{"href":"https://api.github.com/repos/pharo-project/pharo/pulls/3205/comments"},"review_comment":{"href":"https://api.github.com/repos/pharo-project/pharo/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/pharo-project/pharo/pulls/3205/commits"},"statuses":{"href":"https://api.github.com/repos/pharo-project/pharo/statuses/9608f71108926ae83cbe3da3d948d39f2e9f9b82"}},"author_association":"COLLABORATOR"}}
{"actor":{"display_login":"akgrant43"}}