ID:1933973
 
Not a bug
BYOND Version:508
Operating System:Windows 10 Pro
Web Browser:Firefox 40.0
Applies to:DM Language
Status: Not a bug

This is not a bug. It may be an incorrect use of syntax or a limitation in the software. For further discussion on the matter, please consult the BYOND forums.
Descriptive Problem Summary: When a mob uses a verb on an object, the verb leaves a reference to the object even after the verb returns, preventing GC.

Numbered Steps to Reproduce Problem:
1) Create an object that has no references to it. (make_thing())
2) Use a mob (or client) verb to attempt to GC the object by setting it loc to null. (delete_thing_mob())
3) Observe that it fails to do so (check_ref() or list_things())
4) Repeat the experiment using an identical object verb (delete_thing_obj())
5) Observe that it successfully does so

Optionally:

1) Attempt to add A = null at the end of the proc to no success.
2) Attempt to add return at the end of the proc to no success.
3) Use input() instead of an argument to get the object to no success.
4) Replace mob verb with client verb to get the same result.
5) Forcefully delete a mob and observe that the object is now GC'ed.

Code Snippet (if applicable) to Reproduce Problem:
/mob/verb/make_thing()
var/obj/O = new /obj/thing(loc)
world << "[O] made, \ref[O]"


/mob/verb/delete_thing_mob(var/atom/movable/A as null|obj in world)
world << "MD: Ref was \ref[A]"
A.loc = null

/mob/verb/check_ref(var/R as text)
var/atom/A = locate(R)
if(A)
world << "Exists, [A]"
else
world << "Does not exist"

/obj/thing
name = "Thing"

/obj/thing/verb/delete_thing_obj()
set src in view()
world << "OD: Ref was \ref[src]"
loc = null

/mob/verb/list_things()
world << "Listing things"
for(var/atom/movable/A)
world << "[A], \ref[A]"
world << "Done"


Expected Results: Object will be GC'ed as there are no references to it.

Actual Results: Object is left hanging.

Does the problem occur:
Every time? Or how often? Every time.
In other games? It originally occurred in Space Station 13.
In other user accounts? Yes.
On other computers? Yes.

When does the problem NOT occur? When you use object verbs, for example.

Did the problem NOT occur in any earlier versions? If so, what was the last version that worked? (Visit http://www.byond.com/download/build to download old versions for testing.)

Workarounds: Not using mob verbs for objects that need to be GC'ed.

Lummox JR resolved issue (Not a bug)
This actually isn't a bug, although the behavior here is not documented. Here's what's happening:

When a value is used to resolve a verb argument, a routine called AddClientRef() is called. The value is added to a hidden list belonging to the client, internally called owned_refs. This list is scanned periodically and gradually gets cleared out, but the ref is allowed to persist for a while. The object will eventually be soft-deleted after a period of time.

The scan for this is done on the ping timer, which is effectively longer than the max age allowed for those refs (supposed to be 100s). So typically, you can expect a ref that was added in this way to persist for 5 minutes; sometimes less, sometimes a smidge more, depending on the timing.
Why is this list required? Is there some internal reason it can't just be removed?

edit: If it's only verb arguments, why does this also cause it:
3) Use input() instead of an argument to get the object to no success.
input() operates on the same principles internally, so it's still the same deal.

As for why it's required, I think it's mainly to keep the reference intact for a short while after it's been sent to the client so that subsequent parts of the parsing process will work. I don't remember adding that code myself, so I assume that Tom or Dan put it in for a good reason.

The reference will eventually get deleted; on the ping timer, which IIRC is every 5 minutes, the list is scanned and anything older than the max age (which it usually will be by then) is DecRef'd.
Are there any possible workaround besides not using a verb? Does this apply to client, mob, and object verbs or just one set? Or any way to clear the ref out?
It should apply at least to cases where there's a server-side list to choose from (in this case, "in world" did that). There's no way to clear the ref any earlier without deleting the client. It will be cleared when the client is deleted, or when the ref has lived past its allotted time when the ping timer fires.

My question is: Is it super critical that the item in question in fact be deleted right away? If you merely want to avoid it showing up in lists, you could at least do something like change its name to "{deleted object}" so it's obviously a non-choice and it gets pushed to the back of any alphabetical sort.
It has more to do with not wanting random objects to fail to garbage collect and thus being del()'d. Which is really, really slow and wasteful. All because some mob somewhere used a verb on it 3 minutes ago. If the object was actually in a list somewhere, it obviously wouldn't be garbage collecting anyway!

So mob/verb/lookAt(atom/movable/target as mob|obj in view(src)) wouldn't trigger this problem? I assume view, oview, hearers, ohearers, viewers, and oviewers wouldn't trigger this problem.
I believe any of the client-sided lists that can be looked up by DS wouldn't trigger it, because they don't require a call to the server routines that do this.
It has more to do with not wanting random objects to fail to garbage collect and thus being del()'d.

To clarify on this, a lot of ss13 servers use a garbage collection tracking system, that abuses [\ref]'s store a soft reference to the to be deleted object, to be checked during idle cpu time, to see if it GC'ed, and if not, logs its failure to GC than del()'s it.

The current timer is 60 seconds minimum from when the object was dereferenced.

We COULD raise that up to 5 minutes to reduce the false positives in the failed GC log, but it doesn't seem ideal.

Any chance you could look at making the client side code a bit more responsive, if it's the only reference, the server could queue a message to the client(s).

if for no other reason then this behavior could prevent a Del() call on gc from happening on time, leading to a hard to diagnose odd behavior in game code/state, which would only encourage the abuse of del().

(obviously low priority, I'd prefer to see more work done on reducing waits for responses in network code and proc overhead streamlining =P)
I don't think client responsiveness is so much an issue here; it's more a question of why the server needs to hold onto that ref. I'm really not sure. Chances are the TTL for the ref could be lowered dramatically, and probably it could be moved out of the ping timer and into something more responsive (like stat ticks) without any serious cosequences.
Yeah, could it possibly be the ref itself causing the issue?
You make a ref of an item that will be GC'ed and that item gets GC'ed but that ref still exists, like a phantom. Could that be the conflict in it of itself?
AER, the item is not being garbage-collected because it still has a ref.
In response to Lummox JR
Hmmm, is there any way to remove the reference before GC?
I guess I should just stay out of dev help since I never seem to be very good at helping. I like to try to give people different perspectives or ideas, though! My apologies.
Forgive me if I'm wrong but couldn't this reference be a soft reference only, one that doesn't increment or decrement the reference counter?

I don't see how that would cause an issue here considering the fact that the reference already has a system to remove itself albeit after a bit of time has passed.
There's no such concept internally as a soft reference. The refcount is being incremented for a reason, to hold onto the object for longer; I'm just not sure what the exact reasoning behind that is, and I'm loath to mess with it beyond simply lowering the time it persists.
Ah I thought text ref macros were 'soft references'
No, they're not references at all. Their only purpose is for use in locate().
Ah you're right that wouldn't do at all
A soft reference is a reference that doesn't raise the refcounter.

\ref counts.