Compatibility Break Discussion

[BAILOPAN]

pRED and I are mulling over adding a garbage collector to SourceMod. This thread is NOT about whether that's a good idea. What it is about is that it would potentially break extension compatibility, and we'd like input from extension developers.

To GC handles we'd need extensions to implement two functions:
1) For composite handle types (handles that can reference other handles), a callback that asks the IHandleType implementation how to walk the object's memory.
2) For IExtensionInterface, a callback that notifies SourceMod of any Handles being held onto in places it can't see - for example, if your extension has a list of callbacks with "any:data" user values, it would need to inform SourceMod about those.

For example, the code to PJ's SHVector extension would need to walk open vectors and inform SourceMod about each value that is potentially a handle.

If you don't have composite structures, this change is trivial - but it would still break compatibility. In order to get this feature in extensions must use the new API. Otherwise we would have to scan the entire process memory. In worst cases, we would fix all third party extensions. Nevertheless, I'm interested in how everyone feels about this.

Do you think this is a reasonable change? Do you think you'd be able to update your extension? Do you have any questions or thoughts?

This idea is still nascent, but the API would look something like this...
IHandleType changes:

+
+ /**
+ * @brief Returns whether this type is garbage collectible. Return false for
+ * types like SQL connections where the user can "forget" references but
+ * still receive callbacks.
+ *
+ * Note: IsCompositeHandleType must still be implemented even if false.
+ *
+ * @return True if handle is collectible, false otherwise.
+ */
+ virtual bool IsHandleTypeCollectible() =0;
+
+ /**
+ * @brief Returns whether or not this type is a composite type, and
+ * can potentially reference other handles.
+ *
+ * @return Must return true if composite, false otherwise.
+ */
+ virtual bool IsHandleTypeComposite() =0;
+
+ /**
+ * @brief Called when the garbage collector is marking live handles.
+ * If your handle is a composite structure, you must implement this
+ * function and "trace" (inform the GC) about anything that might
+ * possibly be a handle value.
+ *
+ * @param type Handle type.
+ * @param object Handle internal object.
+ * @param tracer Tracer object to use for marking further handles.
+ */
+ virtual void OnTraceHandle(HandleType_t type, void *object, IHandleTracer *tracer) =0;


IHandleTracer, new class:

/**
+ * @brief Informs the garbage collector about reachable handles.
+ */
+ class IHandleTracer
+ {
+ public:
+ /**
+ * @brief Informs the garbage collector about a value that might be a
+ * handle.
+ *
+ * @param value Value that is possibly a handle.
+ */
+ virtual void MaybeTrace(unsigned int value) =0;
+
+ /**
+ * @brief Informs the garbage collector about a value that is
+ * definitely a handle.
+ *
+ * @param handle Handle.
+ */
+ virtual void Trace(Handle_t handle) =0;
+ };
+


IExtensionInterface changes:

+ /**
+ * @brief Called when the GC needs to know about live handles.
+ *
+ * @param tracer Tracer object to inform GC of live handles.
+ */
+ virtual void OnTraceHandles(IHandleTracer *tracer)
+ {
+ }

[Greyscale]

Do you think this is a reasonable change? Do you think you'd be able to update your extension?

Totally dawg.

[Xp3r7]

Im no coder by any means, but what would be the purpose for this "garbage collector" that would break all these extensions?

Just curious. :)

[DJ Tsunami]

I think there's such a low amount of third party extensions that would indeed be trivial to fix them. Some of them (Hacks, Hooker, DukeHacks) can even be deprecated and turned into one big hooks extension.

[BAILOPAN]

Xp3r7: It is a feature for developers that allows them to not worry about leaking memory.

[exvel]

Some of them (Hacks, Hooker, DukeHacks) can even be deprecated and turned into one big hooks extension.
I hope this can be added into the SM 1.3.0 as default functionality. Otherwise such compatibility break will be too harmful for everyone.

[DaFox]

I hope this can be added into the SM 1.3.0 as default functionality. Otherwise such compatibility break will be too harmful for everyone.

That currently is in the works. But the big deal here is that everyone would have to redownload, and repackage extensions (in their plugin zip files) or simply change the code to use the new SDKTools_Hooks.

I think there's such a low amount of third party extensions that would indeed be trivial to fix them.

I have to agree with Tsunami 100%. There is such a small number of extensions that it would be easy to upgrade them, Many of them are not maintained by the original authors but have had derivatives posted by active users (such as Tsunami's dh) which would be upgraded I'm sure. I think internally it would have to be made sure that they are upgraded considering how many plugins rely on them.

I would really like to see this implemented.

[BAILOPAN]

I hope this can be added into the SM 1.3.0 as default functionality.

There are no plans to integrate any third-party extensions as "default functionality". If there are specific features you'd like, feel free to file a bug report.

Please - this thread is for extension developers.

[Kenny Loggins]

So would this break every SourceMod extension?

[sfPlayer]

Imagine the following code which contains a clear runtime error:


new Handle:MySocket;

OnPluginStart() {
MySocket = SocketCreate(SOCKET_TCP, OnSocketError);
SocketConnect(MySocket, OnSocketConnect, OnSocketReceive, OnSocketDisconnect, "localhost", 12345);

someFunction();
someOtherFunction();
}

someFunction() {
SocketClose(MySocket);
}

someOtherFunction() {
SocketSend(MySocket, "blabla");
}

public OnSocketConnect(...
SocketClose will do the following tasks:
1. terminate operation for the specific socket object
2. remove all queued callbacks
3. call something like InvalidateHandle(Handle_t x)

InvalidateHandle must invalidate the handle instantly to make SocketSend in someOtherFunction() throw an error when ReadHandle fails.

In the next GC run (which shouldn't be too late) OnHandleDestroy frees all resources attached to the specific socket.

The socket extension will report all alive socket handles in IExtensionInterface::OnTraceHandles through ITraceHandler::Trace. All data arguments will be reported in IHandleType::OnTraceHandle with ITraceHandler::MaybeTrace.

I also need a legacy interface for old scripts to process CloseHandle similar to the new CloseSocket.

edit: Note that MySocket is global and therefore doesn't drop the reference to the handle, but the Handle should be collected after InvalidateHandle anyway.

[sfPlayer]

An optional argument for handlesys->CreateType to specify the approximate overhead (memory allocated by the extension) per handle could be useful for the GC to determine the actual memory usage.

[BAILOPAN]

sfPlayer: Good idea, that could avoid some perf hits if a general, mostly accurate size of handles is known at creation time.