Tidy up addClassImage API and class <-> image management by jdolan · Pull Request #39 · jdolan/Objectively

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jdolan · 2026-08-18T01:06:49Z

No description provided.

The marker symbol created the failure it then had to guard against: an image could only forget to declare itself because declaring itself was required, and dlsym does not stop at the image it is given, so one that forgot resolved a dependency's marker and was registered under the wrong base address. Guarding that meant asking the loader to relate a handle to an image, which is the platform code the marker was meant to retire. There are only two sources of truth for which image is behind a handle: the loader, or the caller. Take it from the caller. An application loads an image in order to call into it, so it holds an address within that image already, and passing it costs a parameter and no convention. This retires OBJECTIVELY_CLASS_IMAGE, markerBelongsToImage, and the reservation of Objectively as a Class name. Identify class images by an exported marker symbol removeClassImage was handed a handle and had to resolve it to the base address that Classes record, which has no one spelling: Windows hands out the module as the handle, glibc answers from the link map, and macOS, having neither, matched the handle against every loaded image by opening and closing each one in turn. The platforms report a base address for an address, not for a handle. So require an image that provides Classes to say so, with a marker symbol that addClassImage resolves and asks dladdr about once, at registration. removeClassImage then matches on the handle alone and imageForHandle is gone, along with the mach-o and link map includes it needed. The marker is shaped like an archetype and returns NULL, so that a lookup of a Class named Objectively resolves it harmlessly rather than calling something that is not an archetype. dlsym searches an image ahead of its dependencies, so an image resolves its own marker rather than one from the library it links against. The registry becomes a list, which retires MAX_CLASS_IMAGES and the assert that guarded it - a bounds check that compiled out under NDEBUG, leaving the ninth image to write past the array. Registering an image that declares no Classes, or unregistering one that was never registered, now abort. Both were silent, and both leave behind exactly the Classes this exists to remove. Remove the Windows __sync_ shims Nothing calls them since Objectively moved to the __atomic_ builtins, which clang-cl provides directly, so the Interlocked wrappers behind them have no remaining caller. Verify the class image marker belongs to the image dlsym does not stop at the image it is given, so an image that omits OBJECTIVELY_CLASS_IMAGE resolves a dependency's marker instead of failing. It was then registered under the dependency's base address, which made removeClassImage unregister the dependency's Classes and leave its own behind, reachable by name and about to be unmapped - the failure the marker exists to prevent, reached silently. Confirm the marker was defined by the image behind the handle, by asking which image defines it and reopening that one RTLD_NOLOAD to compare. Windows hands out the module as the handle, so there the base address answers directly. Retire image entries in place rather than unlinking and freeing them. A concurrent classForName walks this list, and freeing a node out from under it turned a stale read into a use after free. Retired entries are skipped on lookup and freed at teardown. Also tie the marker's definition to its lookup through one macro, so the two cannot drift, and drop the remark claiming an image declares nothing. Order Class.c to follow Class.h removeClassImage preceded addClassImage because imageForHandle, its only helper, sat directly above it. That helper is gone, and the pair had been left reading backwards with markerBelongsToImage stranded between them. Definitions now follow the order the header declares them in, and each static helper sits directly above its first caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors how Objectively tracks dynamically loaded “class images” (e.g., plugins) by making image identification explicit via an in-image address, and updates the internal bookkeeping used by classForName and image removal.

Changes:

Updated addClassImage API to accept both a dlopen handle and a trusted in-image address used to resolve the image base.
Replaced the fixed-size image registry with a linked-list of registered images and adjusted lookup/removal behavior accordingly.
Removed unused __sync_* interlock shims from the VS15 compatibility layer.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
Sources/Objectively/Class.h	Updates `addClassImage` signature and clarifies documentation around image/address-based registration and removal behavior.
Sources/Objectively/Class.c	Implements address-to-image-base resolution and replaces the image registry with a linked list used by `classForName` and `removeClassImage`.
Objectively.vs15/Sources/Windowly.h	Removes now-unused `__sync_*` declarations/macros.
Objectively.vs15/Sources/Windowly.c	Removes now-unused `__sync_*` wrapper implementations; minor region pragma alignment.

Suppressed comments (2)

Sources/Objectively/Class.c:262

removeClassImage mutates i->handle / i->image without any synchronization while classForName may read them. Even if nodes are never freed, these plain reads/writes are a data race in C. Use atomic loads/stores for the head pointer and for retiring a node’s fields.

 for (ClassImage *i = _images; i; i = i->next) {
if (i->handle == handle) {
image = i->image;
i->handle = NULL;
i->image = NULL;

Sources/Objectively/Class.c:305

classForName walks _images via non-atomic loads and reads i->handle directly. With concurrent addClassImage/removeClassImage, this can observe a partially-published node or race with retirement stores. Load the list head and each node’s handle atomically (acquire) before calling dlsym.

 for (ClassImage *i = _images; i && archetype == NULL; i = i->next) {
if (i->handle) {
archetype = dlsym(i->handle, s);
}
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

classForName walks the image list on any thread, while addClassImage prepended to it and removeClassImage wrote through it with plain stores. Two registrations could lose one another, and a lookup could read a half-written node. Push with a compare and swap, as a Class is pushed, and retire an image with a single release store of the handle the walk matches on, so that walk sees an image or does not and never part of one. Nothing is unlinked or freed before teardown, so a walk in progress always has a next. removeClassImage's unlinking of Classes is still not atomic against a concurrent registration, and two concurrent calls still race with each other. Both remain the caller's to serialize, as documented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

removeClassImage splices _classes while classForName walks it. Unlinking is a read and a write over a list another thread is traversing, and making each store atomic does not make the pair of them one operation: the head store could drop a registration published between them, and clearing next could end a live walk early, hiding every Class behind it. A mutex around the three operations on the list, taken only for the walk, the push and the splice, and never across dlsym, dlopen or a Class initializer, each of which can reenter _initialize. The atomics on _classes go away with it. _images keeps its atomics. It is only ever pushed to and retired in place, never spliced, and it is walked while calling dlsym, which the lock must not cover. Verified under ThreadSanitizer, six threads looking up names against two thousand register and unregister cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retiring an image and dropping its Classes were two steps with a gap between them, so two calls for the same handle could both find it registered, both proceed, and neither report the duplicate the abort is there to catch. Hold the lock from the search through the last unlink. Removing two different images was already safe, since each retires its own entry and the unlinking was already serialized. This closes the case of the same handle arriving twice, and makes that abort exact. Nothing in this call reaches the loader, so the lock may cover all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

CopilotAI lite review requested due to automatic review settings August 18, 2026 01:06

Copilotstarted reviewing on behalf of jdolan August 18, 2026 01:07 View session

CopilotAI reviewed Aug 18, 2026
View reviewed changes

Comment threadSources/Objectively/Class.c Outdated

jdolanand others added 4 commits August 17, 2026 21:15

Delete stale test.
07ecf78

jdolan merged commit fbb3f8c into mainAug 18, 2026
4 checks passed

jdolan deleted the cleanup/class-images branch August 18, 2026 01:39

Uh oh!

Conversation

jdolan commented Aug 18, 2026

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jdolan commented Aug 18, 2026

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jdolan commented Aug 18, 2026

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jdolan commented Aug 18, 2026

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jdolan commented Aug 18, 2026

Uh oh!